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.

Sign up for GitHub

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

Jump to bottom

next/198/20231121/v1 #9853

Merged
merged 5 commits into from
Nov 21, 2023
Merged

next/198/20231121/v1 #9853

merged 5 commits into from
Nov 21, 2023

Conversation

victorjulien
Copy link
Member

catenacyber and others added 5 commits November 20, 2023 21:53
@catenacyber
output/alert: rewrite code for app-layer properties
0b6b015 
Especially fix setup-app-layer script to not forget this part

This allows, for simple loggers, to have a unique definition
of the actual logging function with the jsonbuilder.
This way, alerts, files, and app-layer event can share the code
to output the same data.

Ticket: OISF#3827
@catenacyber
output/ftp: have ftp properties in alerts
3b1b163 
Ticket: 6500
@catenacyber
output/tftp: have tftp properties in alerts
8a09bff 
Ticket: 6501
@catenacyber
output/krb5: have krb5 properties in alerts
e38b9de 
Ticket: 5977
@catenacyber @victorjulien
detect: SigMatchAppendSMToList can fail
c272a64 
Ticket: OISF#6104

And failures should be handled to say that the rule failed to load

Reverts the fix by 299ee6e
that was simple, but not complete (memory leak),
to have this bigger API change which simplifies code.
@codecov Codecov
Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Merging #9853 (c272a64) into master (90c1765) will decrease coverage by 0.05%.
The diff coverage is 81.85%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #9853      +/-   ##
==========================================
- Coverage   82.47%   82.42%   -0.05%     
==========================================
  Files         973      973              
  Lines      273962   273261     -701     
==========================================
- Hits       225944   225230     -714     
- Misses      48018    48031      +13
Flag Coverage Δ
fuzzcorpus 64.25% <75.94%> (-0.13%) ⬇️
suricata-verify 61.07% <61.81%> (-0.04%) ⬇️
unittests 62.92% <39.87%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 16667

jufajardini
jufajardini approved these changes Nov 21, 2023
View reviewed changes
Copy link
Contributor

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing seems to be missing, left over or surplus.

@victorjulien victorjulien merged commit c272a64 into OISF:master Nov 21, 2023
This was referenced Nov 21, 2023
Merged
Closed
@victorjulien victorjulien deleted the next/198/20231121/v1 branch June 23, 2024 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jufajardini jufajardini jufajardini approved these changes

@catenacyber catenacyber Awaiting requested review from catenacyber catenacyber is a code owner

@jasonish jasonish Awaiting requested review from jasonish jasonish is a code owner

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@victorjulien @suricata-qa @jufajardini @catenacyber