-
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
Detect validate callbacks 5634 v3 #11902
Detect validate callbacks 5634 v3 #11902
Conversation
Ticket: 5634 Just an optimization to have less code, and to later have an easier way for rust keywords to use it
Ticket: 5634
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11902 +/- ##
==========================================
+ Coverage 82.60% 82.62% +0.01%
==========================================
Files 912 912
Lines 249342 249181 -161
==========================================
- Hits 205968 205875 -93
+ Misses 43374 43306 -68
Flags with carried forward coverage won't be shown. Click here to find out more. |
Information: QA ran without warnings. Pipeline 23042 |
@@ -1315,7 +1316,20 @@ bool DetectEngineBufferRunValidateCallback( | |||
{ | |||
const DetectBufferType *map = DetectEngineBufferTypeGetById(de_ctx, id); | |||
if (map && map->ValidateCallback) { | |||
return map->ValidateCallback(s, sigerror); |
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.
this logic seems to turn a generic validation callback mechanism into a specific one for content. That does not look correct.
bool supports_transforms; | ||
bool multi_instance; /**< buffer supports multiple buffer instances per tx */ | ||
void (*SetupCallback)(const struct DetectEngineCtx_ *, struct Signature_ *); | ||
bool (*ValidateCallback)( |
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.
validation should be possible for non-content as well
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.
Interesting, what can we validate besides content ?
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.
all other keywords :)
examples would be bsize, byte*, etc.
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.
Ok, so we could have DetectMd5ValidateCallback
forbid the usage of other stuff than content
for instance... Interesting
(As of today, we only use content though, right ?)
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.
see inline
Next version in #11963 |
Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/5634
Describe changes:
#11699 with review taken into account :
This was done on the way to convert quic keywords to rust like #11575
With this PR, it should be easy to add a wrapper for rust for this ValidateCallback stuff