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

next/682/20250108/v1 #12358

Merged
merged 18 commits into from
Jan 9, 2025
Merged

Conversation

jlucovsky and others added 18 commits January 8, 2025 17:06
Issue: 7466

This commit releases the memory for the flow variable "key" when
the flow variable is of type string. The key is allocated in the Lua
extension logic.
So we get:
1. request arrives - buffered due to not ackd
2. response arrives, acks request - request is now parsed, response isn't
3. ack for response, response parsed. Then detect runs for request,
generates alert. We now have 2 txs. txid will be 0 from AppLayerParserGetTransactionInspectId

But txid 1 is unidirectional in the other way, so we can use txid 0
metadata for logging

Ticket: 7449
Even if they get defined
to make scan-build happy avoiding its warning :

Excessive padding in 'struct DetectEngineThreadCtx_'
(33 padding bytes, where 1 is optimal)
to avoid timeouts

instead of forbidding pcre signatures on stream

Ticket: 4858
instead of a global variable.

For easier initialization with dynamic number of protocols
for expectation_proto

Ticket: 5053
so that we can use safely EXCEPTION_POLICY_MAX*sizeof(x)
Ticket: 5053

delay after initialization so that StringToAppProto works
Ticket: 7465

If a bug chunk of data is parsed in one go, we could create many
transactions even if marking them as complete, and have
quadratic complexity calling find_request.

Proposed solution is to fail on creating a new transaction if too
many already exist.
@victorjulien victorjulien requested review from jasonish, jufajardini and a team as code owners January 8, 2025 18:57
Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 88.01653% with 29 lines in your changes missing coverage. Please review.

Project coverage is 82.54%. Comparing base (def22fa) to head (494d7bf).
Report is 18 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12358      +/-   ##
==========================================
- Coverage   83.23%   82.54%   -0.69%     
==========================================
  Files         912      912              
  Lines      257647   258028     +381     
==========================================
- Hits       214450   212988    -1462     
- Misses      43197    45040    +1843     
Flag Coverage Δ
fuzzcorpus 60.72% <75.10%> (-0.50%) ⬇️
livemode 19.39% <54.81%> (-0.01%) ⬇️
pcap 44.42% <73.64%> (+0.02%) ⬆️
suricata-verify 63.19% <85.35%> (+0.32%) ⬆️
unittests 58.11% <55.41%> (-1.08%) ⬇️

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

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.

Staging looks OK.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24124

@victorjulien victorjulien deleted the next/682/20250108/v1 branch January 9, 2025 05:47
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.

5 participants