-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[Draft] Prototype approach for issue 3616 #5075
Conversation
src/detect.c
Outdated
@@ -1486,6 +1507,8 @@ static void DetectRunTx(ThreadVars *tv, | |||
tx.tx_ptr, tx.tx_id, | |||
flow_flags & STREAM_TOSERVER ? "toserver" : "toclient", | |||
new_detect_flags); | |||
if (alproto == ALPROTO_HTTP) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer a solution where we can avoid adding checks here.
One way would be to make sure HttpServerBodyGetDataCallback
is still called only once, but we cache the output for the duration of the current run. DetectEngineInspectBufferHttpHeader
does something similar for HTTP headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though I have to say I don't really like the solution http_header
uses either. But it does put the cost where it belongs, in the keyword implementation. This way its not a cost that all the other keywords and protocols also have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@victorjulien This implements your second suggestion -- please review when you have time.
Is there a S-V test for this ? |
@jlucovsky do we have an SV test? |
|
|
e37de7e
to
1b7e0e1
Compare
Update HTTP content body after all transactions have been handled. This is to fix the issue described in 3616. This is a draft PR and cleanup/reorganization/etc is needed. This PR serves as a prototype approach for a solution.
Continued in #5549 |
Implements ticket feature OISF#5075
Fix feature in ticket OISF#5075
Ticket: OISF#5075 Signed-off-by: jason taylor <jtfas90@gmail.com>
Ticket: OISF#5075 Signed-off-by: jason taylor <jtfas90@gmail.com>
Link to redmine ticket: 3616
Describe changes:
suricata-verify-pr: 348
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch: