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

Libhtp rs v27.3 #12538

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Libhtp rs v27.3 #12538

wants to merge 4 commits into from

Conversation

catenacyber
Copy link
Contributor

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

Describe changes:

  • prepare libhtp-rs by minimizing the diff for src subdir

This allow to go one small step further

#12509 next round now focusing on src/app-layer-htp.c

In preparation of libhtp rust
In preparation of libhtp rust
@suricata-qa
Copy link

ERROR:

ERROR: QA failed on build_asan.

Pipeline 24648

@suricata-qa
Copy link

ERROR:

ERROR: QA failed on build_asan.

Pipeline 24654

Copy link

codecov bot commented Feb 6, 2025

Codecov Report

Attention: Patch coverage is 95.93496% with 5 lines in your changes missing coverage. Please review.

Project coverage is 80.71%. Comparing base (d4330ef) to head (0c48705).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12538      +/-   ##
==========================================
+ Coverage   80.68%   80.71%   +0.02%     
==========================================
  Files         925      925              
  Lines      258914   258929      +15     
==========================================
+ Hits       208914   208987      +73     
+ Misses      50000    49942      -58     
Flag Coverage Δ
fuzzcorpus 56.89% <85.83%> (+0.06%) ⬆️
livemode 19.40% <11.66%> (-0.01%) ⬇️
pcap 44.22% <85.00%> (+0.02%) ⬆️
suricata-verify 63.39% <88.33%> (-0.01%) ⬇️
unittests 58.38% <88.61%> (+<0.01%) ⬆️

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

{ "RESPONSE_FIELD_MISSING_COLON", HTTP_DECODER_EVENT_RESPONSE_FIELD_MISSING_COLON },
{ "INVALID_REQUEST_CHUNK_LEN", HTTP_DECODER_EVENT_INVALID_REQUEST_CHUNK_LEN },
{ "INVALID_RESPONSE_CHUNK_LEN", HTTP_DECODER_EVENT_INVALID_RESPONSE_CHUNK_LEN },
{ "UNKNOWN_ERROR", HTP_LOG_CODE_UNKNOWN },
Copy link
Member

Choose a reason for hiding this comment

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

Is the change from the HTTP prefix to HTP a change that was made in the libhtp Rust conversion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, it is a change that vill be made cf #12508 commit message :

libhtp logging API updated to provide HTP_LOG_CODE constants along with
the message. This eliminates the need to perform string matching on
message text to map log messages to HTTP_DECODER_EVENT values, and the
HTP_LOG_CODE values can be used directly. In support of this,
HTP_DECODER_EVENT values are mapped to their corresponding HTP_LOG_CODE
values:

HTTP_DECODER_EVENT_UNKNOWN_ERROR => HTP_LOG_CODE_UNKNOWN

In preparation of libhtp rust
In preparation of libhtp rust
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24661

Copy link
Member

@jasonish jasonish left a comment

Choose a reason for hiding this comment

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

I think it looks OK, and clean PR history (no force pushes) may be requested tho.

Copy link
Member

@victorjulien victorjulien left a comment

Choose a reason for hiding this comment

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

Looks OK to me, just need a clean PR.

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.

4 participants