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

signatures: Support custom event via [event_name] syntax #3478

Merged
merged 4 commits into from
Dec 5, 2023

Conversation

awelzel
Copy link
Contributor

@awelzel awelzel commented Nov 29, 2023

This change allows to specify a per signature specific event, overriding
the default signature_match event. It further removes the message
parameter from such events if not provided in the signature.

Closes #3403


zeek-docs: zeek/zeek-docs#234

@awelzel awelzel added the Docs: required New functionality or behavior that should be covered in our documentation label Nov 29, 2023
@awelzel awelzel force-pushed the topic/awelzel/3403-signature-match-event branch from 64d2473 to 67b4214 Compare November 29, 2023 19:47
@awelzel awelzel requested a review from vpax November 29, 2023 20:03
@awelzel awelzel marked this pull request as ready for review November 29, 2023 20:04
@awelzel awelzel force-pushed the topic/awelzel/3403-signature-match-event branch 2 times, most recently from ad94802 to 8a3caec Compare December 4, 2023 15:33
@awelzel awelzel requested a review from timwoj December 4, 2023 15:33
@awelzel awelzel force-pushed the topic/awelzel/3403-signature-match-event branch from 8a3caec to ff582e0 Compare December 4, 2023 15:34
@awelzel
Copy link
Contributor Author

awelzel commented Dec 4, 2023

@timwoj , mind looking at this one. @vpax , this is what was discussed on #3403 , maybe want to skim? Usage looks reasonably nice (given the constraints).

src/RuleAction.cc Show resolved Hide resolved
src/RuleAction.cc Show resolved Hide resolved
src/RuleAction.cc Outdated Show resolved Hide resolved
src/RuleAction.h Show resolved Hide resolved
NEWS Show resolved Hide resolved
@vpax
Copy link
Contributor

vpax commented Dec 5, 2023

@awelzel looks good, I just have some micro-questions, per what I've now entered above

When trying to use TOK_IDENT and TOK_STRING in a single rule, that
resulted in "corrupt" strings.

https://www.gnu.org/software/bison/manual/html_node/Strings-are-Destroyed.html
With custom events for signatures, Reporter::error() may be invoked
while loading them. Early exit in case that happens. We could continue
and either disable the signatures or fallback to the default
signature_match() event, but not sure that would be obviously better.
This change allows to specify a per signature specific event, overriding
the default signature_match event. It further removes the message
parameter from such events if not provided in the signature.

This also tracks the message as StringValPtr directly to avoid
allocating the same StringVal for every DoAction() call.

Closes #3403
And return const std::string& from GetMIME(). Probably not at all performance
relevant, but while I'm already here.
@awelzel awelzel force-pushed the topic/awelzel/3403-signature-match-event branch from ff582e0 to e8241e1 Compare December 5, 2023 14:29
@vpax
Copy link
Contributor

vpax commented Dec 5, 2023

@awelzel thanks for the clarifications, LGTM

@awelzel awelzel merged commit efc6918 into master Dec 5, 2023
31 checks passed
@awelzel awelzel deleted the topic/awelzel/3403-signature-match-event branch December 5, 2023 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs: required New functionality or behavior that should be covered in our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More flexible signature "event" interface
3 participants