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

template: move app-layer registration code to rust #11816

Closed

Conversation

catenacyber
Copy link
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/3195

Describe changes:

  • template: move app-layer registration code to rust

#11726 with needed rebase after SIP transition to rust

I would also remove ./src/detect-template.c as it looks obsolete (by detect-template2)

Ticket: 3195

Also remove unused src/tests/detect-template-buffer.c
Copy link

codecov bot commented Sep 23, 2024

Codecov Report

Attention: Patch coverage is 42.18750% with 37 lines in your changes missing coverage. Please review.

Project coverage is 82.60%. Comparing base (7ab8334) to head (c981804).
Report is 33 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11816      +/-   ##
==========================================
+ Coverage   82.58%   82.60%   +0.02%     
==========================================
  Files         914      914              
  Lines      249500   249456      -44     
==========================================
+ Hits       206045   206069      +24     
+ Misses      43455    43387      -68     
Flag Coverage Δ
fuzzcorpus 60.58% <42.18%> (+0.14%) ⬆️
livemode 18.87% <7.81%> (+0.15%) ⬆️
pcap 44.09% <7.81%> (+0.02%) ⬆️
suricata-verify 62.02% <7.81%> (-0.01%) ⬇️
unittests 58.97% <7.81%> (+0.01%) ⬆️

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

@victorjulien
Copy link
Member

IIRC template vs template2 was about a buffer keyword vs a packet match? Might be good to have examples for both.

@catenacyber
Copy link
Contributor Author

IIRC template vs template2 was about a buffer keyword vs a packet match? Might be good to have examples for both.

Nope : template and template2 are both packet match, and detect-template-rust-buffer.c is the buffer one (which I move from C to rust in this PR)

@victorjulien
Copy link
Member

IIRC template vs template2 was about a buffer keyword vs a packet match? Might be good to have examples for both.

Nope : template and template2 are both packet match, and detect-template-rust-buffer.c is the buffer one (which I move from C to rust in this PR)

Ok, then we should probably consolidate.

@catenacyber
Copy link
Contributor Author

IIRC template vs template2 was about a buffer keyword vs a packet match? Might be good to have examples for both.

Nope : template and template2 are both packet match, and detect-template-rust-buffer.c is the buffer one (which I move from C to rust in this PR)

Ok, then we should probably consolidate.

I created https://redmine.openinfosecfoundation.org/issues/7278 for this

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.uptime 649 628 96.76%

Pipeline 22884

let tx = cast_pointer!(tx, TemplateTransaction);
if flags & Direction::ToClient as u8 != 0 {
if let Some(ref response) = tx.response {
if !response.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

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

not new in this PR, but I wonder if this is correct. Should be able to match on bsize:0; but we won't be returning an 0 sized buffer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right

return;
}
/* TEMPLATE_END_REMOVE */
// TODO create a suricata-verify test
Copy link
Member

Choose a reason for hiding this comment

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

todo for this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rather for new keywords getting created

But OISF/suricata-verify#2087 and thanks for finding new bugs

@victorjulien
Copy link
Member

Looks good overall, some comments / questions inline

@catenacyber
Copy link
Contributor Author

Next in #11911

@catenacyber catenacyber closed this Oct 8, 2024
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