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

smtp: check if there is a transaction to close #7050

Closed

Conversation

catenacyber
Copy link
Contributor

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

Describe changes:

  • smtp: check if there is a transaction to close (for starttls)

Ticket: 4948

When parsing the response for starttls
@catenacyber catenacyber requested a review from a team as a code owner February 22, 2022 12:12
@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #7050 (9c356e1) into master (b1c0936) will decrease coverage by 2.41%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #7050      +/-   ##
==========================================
- Coverage   77.88%   75.46%   -2.42%     
==========================================
  Files         628      628              
  Lines      185373   185243     -130     
==========================================
- Hits       144374   139792    -4582     
- Misses      40999    45451    +4452     
Flag Coverage Δ
fuzzcorpus 54.48% <100.00%> (-4.02%) ⬇️
suricata-verify 54.44% <100.00%> (-0.11%) ⬇️
unittests 63.17% <100.00%> (+<0.01%) ⬆️

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 6397

@inashivb
Copy link
Member

Do we have an s-v test as mentioned on the issue for this? The change looks fine on the first sight.

@catenacyber
Copy link
Contributor Author

Do we have an s-v test as mentioned on the issue for this? The change looks fine on the first sight.

There is none for the moment.
I confess I rely on oss-fuzz for regressions testing on the issues it finds
I could craft one S-V test if needed out of the oss-fuzz reproducer

@victorjulien
Copy link
Member

Do we have an s-v test as mentioned on the issue for this? The change looks fine on the first sight.

There is none for the moment. I confess I rely on oss-fuzz for regressions testing on the issues it finds I could craft one S-V test if needed out of the oss-fuzz reproducer

Can you craft such a test?

@catenacyber
Copy link
Contributor Author

Can you craft such a test?

Done in OISF/suricata-verify#784

@jasonish jasonish assigned inashivb and unassigned inashivb Mar 24, 2022
This was referenced Mar 25, 2022
@victorjulien
Copy link
Member

Merged in #7172, thanks!

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