-
Notifications
You must be signed in to change notification settings - Fork 90
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
tests: Updates for 6555 #2012
tests: Updates for 6555 #2012
Conversation
match: | ||
event_type: alert | ||
alert.signature_id: 1 | ||
payload_printable: "GET /1 HTTP/1.0\r\nUser-Agent: Mozilla\r\n\r\n" | ||
payload_length: 40 | ||
- filter: | ||
count: 1 | ||
version: 7.0.7 | ||
lt-version: 8.0 |
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.
You do not need lt-version: 8.0 here, do you ?
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.
Yes -- this check if for main-7.0.x with the backport so [7.0.7, 8.0)
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.
Why does it not work for 8 ?
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.
The test for 8.0 checks for the recently-added payload_length
(which isn't in main-7.0.x)
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.
So, this check also works for 8, you do not need lt-version, 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.
That's true but there is a filter check immediately preceding this one that is targeted for 8+
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.
That gives me the impression that this check will fail for 8, so I would remove the lt-version
Anyways, this is a style nit...
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.
Will update.
match: | ||
event_type: alert | ||
alert.signature_id: 1 | ||
payload_printable: "GET /1 HTTP/1.0\r\nUser-Agent: Mozilla\r\n\r\nGET /2 HTTP/1.0\r\nUser-Agent: Mozilla\r\n\r\n" | ||
payload_length: 80 | ||
- filter: | ||
count: 1 | ||
version: 7.0.7 |
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.
More of these.
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 comments inline.
This commit provides updates needed for issue 6555. Previously, the gap handling was restricted to master; 6555 adds those changes to main-7.0.x Most of the changes are to extend the version; the eve-payload-07-http-gap tests adds version-based checks as a new output value payload_length is not available in main-7.0.x
Please open a new PR, this one is a bit messy now. |
Continued in #2038 |
This commit provides updates needed for issue 6555. Previously, the gap handling was restricted to master; 6555 adds those changes to main-7.0.x
Most of the changes are to extend the version; the eve-payload-07-http-gap tests adds version-based checks as a new output value payload_length is not available in main-7.0.x
Ticket
If your pull request is related to a Suricata ticket, please provide
the full URL to the ticket here so this pull request can monitor
changes to the ticket status:
Redmine ticket: https://redmine.openinfosecfoundation.org/issues/6155