Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/http range/v24 #6331

Closed
wants to merge 10 commits into from

Conversation

catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/1576

Describe changes:

  • Uses a generic container (a thread-safe hash table whose key is the url/filename) to store file parts with HTTP Range header
  • handles reassembly of these parts, while enforcing a global memcap

suricata-verify-pr: 517
OISF/suricata-verify#517

Continues #6312 with using FileContainer pointer instead of boolean fileOwned or fileOwning

For testing : https://github.com/jasonish/suricata-http-range-test

Left TODO : implement for HTTP2

victorjulien and others added 10 commits September 2, 2021 17:17
Compares two buffers with their two sizes
adds a container, ie a thread safe hash table whose
key is the filename

keep a tree of unordered ranges, up to a memcap limit

adds HTPFileOpenWithRange to handle like HTPFileOpen
if there is a range : open 2 files, one for the whole reassembled,
and one only for the current range
Simplify locking by using the THashData lock instead of a separate
range lock.

Avoid size_t in function arguments.

Clean up file handling functions.

Implement handling of alloc errors.

Rename yaml entry to byterange

Unify public api naming
Better structure design to ensure that one flow maximum
is owning and appending into the file, adding fileOwning field.

Adds also a gap field in a range buffer, so that we can
feed the gap on closing, when we are protected from concurrency
by a lock, (lock which got removed in the append path)

Fixes memcap when encountering a duplicate while inserting
in red and black tree

Adds many comments
To make concurrency reasoning clearer
@catenacyber catenacyber marked this pull request as ready for review September 2, 2021 15:25
@catenacyber catenacyber requested a review from a team as a code owner September 2, 2021 15:25
@catenacyber catenacyber force-pushed the feature/http-range/v24 branch from 6985b81 to dc12989 Compare September 2, 2021 15:37
@catenacyber
Copy link
Contributor Author

Replaced by #6333

@catenacyber catenacyber closed this Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants