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

Multipart mime 3487 v4 #1383

Closed
wants to merge 1 commit into from

Conversation

catenacyber
Copy link
Collaborator

Ticket

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

See OISF/suricata#9424

#1377 with review taken into account
@inashivb do you approve ?

@catenacyber catenacyber added the tests pass These new tests should pass label Sep 11, 2023
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.

Some nits inline.
I approve 💯 . Bring me more tests! 😄


## PCAP

Previous unit test for MIME in Suricata
Copy link
Member

Choose a reason for hiding this comment

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

They all say this but have a pcap within every test. Can we deduplicate it?
createst cannot do this at the moment. It copies pcap files to the test..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are different unit tests / pcaps...
Do I miss something ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming she thought they were all the same... 🤔 maybe just some rewording.
Maybe: Adapted using data from a previous specific unit test for MIME in Suricata. ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@@ -0,0 +1,48 @@
# *** Add configuration here ***
Copy link
Member

Choose a reason for hiding this comment

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

nit: These are indicators to add more conf like min version required and should/can be removed when no additional conf is needed

Copy link
Member

Choose a reason for hiding this comment

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

ah. i see its done in the next commit. I think these fixes should be a part of this commit.

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
@catenacyber
Copy link
Collaborator Author

Replaced by #1385

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.

3 participants