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

Output alert applayer v17.1 #9870

Closed

Conversation

catenacyber
Copy link
Contributor

@catenacyber catenacyber commented Nov 23, 2023

Link to redmine tickets:
https://redmine.openinfosecfoundation.org/issues/3827
Preliminary work for https://redmine.openinfosecfoundation.org/issues/5053

Describe changes:

  • output: unify boilerplate code (trying to become a lines-of-code neutral contributor to Suricata ;-) and rising the percentage of rust files )
  • output/dns: do not add empty app-layer metadata
  • app-layer: do not require probing parser as fixed patterns can be enough
  • dnp3: restrict function scope to one file

There is one behavior change for SSH switching from LOG_DIR_PACKET to LOG_DIR_FLOW
What do you think about it ?

Follow up after large of #9851 (part of #9812 for app-layer plugins)
#9869 with right commit pushed

@jasonish what do you think about the DNS commit ?

Should I split this into multiple PRs ?

SV_BRANCH=pr/1490

OISF/suricata-verify#1490

Copy link

codecov bot commented Nov 23, 2023

Codecov Report

Merging #9870 (55216df) into master (41c0526) will increase coverage by 0.01%.
The diff coverage is 98.42%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9870      +/-   ##
==========================================
+ Coverage   82.45%   82.47%   +0.01%     
==========================================
  Files         973      961      -12     
  Lines      273063   272672     -391     
==========================================
- Hits       225155   224875     -280     
+ Misses      47908    47797     -111     
Flag Coverage Δ
fuzzcorpus 64.48% <79.52%> (+0.12%) ⬆️
suricata-verify 61.02% <98.42%> (-0.07%) ⬇️
unittests 63.00% <37.79%> (+0.08%) ⬆️

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 16694

@jasonish
Copy link
Member

@jasonish what do you think about the DNS commit ?

Looks fine, this is good.

@catenacyber
Copy link
Contributor Author

Rebased in #10166

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants