-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
template: move app-layer registration code to rust #11816
template: move app-layer registration code to rust #11816
Conversation
Ticket: 3195 Also remove unused src/tests/detect-template-buffer.c
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11816 +/- ##
==========================================
+ Coverage 82.58% 82.60% +0.02%
==========================================
Files 914 914
Lines 249500 249456 -44
==========================================
+ Hits 206045 206069 +24
+ Misses 43455 43387 -68
Flags with carried forward coverage won't be shown. Click here to find out more. |
IIRC template vs template2 was about a buffer keyword vs a packet match? Might be good to have examples for both. |
Nope : template and template2 are both packet match, and detect-template-rust-buffer.c is the buffer one (which I move from C to rust in this PR) |
Ok, then we should probably consolidate. |
I created https://redmine.openinfosecfoundation.org/issues/7278 for this |
WARNING:
Pipeline 22884 |
let tx = cast_pointer!(tx, TemplateTransaction); | ||
if flags & Direction::ToClient as u8 != 0 { | ||
if let Some(ref response) = tx.response { | ||
if !response.is_empty() { |
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.
not new in this PR, but I wonder if this is correct. Should be able to match on bsize:0;
but we won't be returning an 0 sized buffer?
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.
right
return; | ||
} | ||
/* TEMPLATE_END_REMOVE */ | ||
// TODO create a suricata-verify test |
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.
todo for this PR?
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.
Rather for new keywords getting created
But OISF/suricata-verify#2087 and thanks for finding new bugs
Looks good overall, some comments / questions inline |
Next in #11911 |
Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/3195
Describe changes:
#11726 with needed rebase after SIP transition to rust
I would also remove
./src/detect-template.c
as it looks obsolete (by detect-template2)