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

mime: add previous suricata unit tests #1385

Closed
wants to merge 2 commits into from

Conversation

catenacyber
Copy link
Collaborator

Ticket

Redmine ticket: https://redmine.openinfosecfoundation.org/issues/3487

See OISF/suricata#9424

#1383 with review taken into account
@inashivb do you approve the changes to bug-6207 ?

mime: fix tests for bug-6207

Fix manually crafted pcaps to have valid MIME headers folding
beginning with space

And removing the test for BODY_BOUND which is becoming obsolete
@jufajardini
Copy link
Contributor

Thanks for adding more info about pcap origin and which unittest has been converted! :)

Copy link
Member

@inashivb inashivb left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks! :)
Q: w.r.t changes made in pcaps and the corresponding Suricata nom parsing: Are we always guaranteed to receive specific number of space chars in between headers?

@catenacyber
Copy link
Collaborator Author

Are we always guaranteed to receive specific number of space chars in between headers?

I do not think so.

This is about headers folding.
Headers are name: value on each line
If you want a multi line value, you have to start the next line with a space like

HeaderName: Header value and
 continuing on this line
NewHeaderName: newValue

You can see how Wireshark interprets differently your pcap and mine ;-)

@catenacyber catenacyber added the tests pass These new tests should pass label Sep 18, 2023
@jasonish jasonish mentioned this pull request Sep 19, 2023
@jasonish jasonish mentioned this pull request Sep 28, 2023
@victorjulien
Copy link
Member

Merged in #1385, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests pass These new tests should pass
Development

Successfully merging this pull request may close these issues.

4 participants