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

Improve fuzzer #1191

Merged
merged 2 commits into from
Apr 24, 2024
Merged

Improve fuzzer #1191

merged 2 commits into from
Apr 24, 2024

Conversation

tyler92
Copy link
Contributor

@tyler92 tyler92 commented Feb 12, 2024

The current fuzzer target covered only basic things and didn't cover the most interesting parts of HTTP request parsing. I've extended fuzzer with more targets and added a corpus. As a result, coverage has been increased significantly and several memory issues have been found; I will report them separately to avoid mixing them with this MR.

Notes:

  1. Due to the reuse policy, I had to add ".license" files for each file in the corpus. I'm unsure if it's a good idea and would appreciate your suggestions.
  2. The "router" target can't be covered via public interface because the Router class requires dependencies that send messages over a network. So Router is covered via the SegmentTreeNode class.
  3. The 'removeRoute' method is excluded from the fuzzer due to crashes (there are no null pointer checks). Probably we can enable it later when it's fixed

Copy link

codecov bot commented Feb 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.09%. Comparing base (7cc8732) to head (5dba79a).
Report is 5 commits behind head on master.

❗ Current head 5dba79a differs from pull request most recent head 89cf458. Consider uploading reports for the commit 89cf458 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1191      +/-   ##
==========================================
- Coverage   78.09%   78.09%   -0.01%     
==========================================
  Files          53       53              
  Lines        7082     7077       -5     
==========================================
- Hits         5531     5527       -4     
+ Misses       1551     1550       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kiplingw
Copy link
Member

Good work @tyler92. We'll get back to you as soon as we've had a chance to review.

@Tachi107
Copy link
Member

Hi @tyler92, thanks for the patch! Fuzzing is definitely an interesting area where we could do better.

Due to the reuse policy, I had to add ".license" files for each file in the corpus. I'm unsure if it's a good idea and would appreciate your suggestions.

What you can do instead is add a .reuse/dep5 file in the project root which contains licensing information for all those small files, for example:

Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/

Files: tests/fuzzers/corpus/http-*.txt
Copyright: 2024 Mikhail Khachayants
License: Apache-2.0

@tyler92
Copy link
Contributor Author

tyler92 commented Mar 27, 2024

@Tachi107 Thanks for the hint with .reuse/dep5, it works!

@tyler92
Copy link
Contributor Author

tyler92 commented Apr 24, 2024

@kiplingw Just in case - there are no more changes expected on my part, the MR is ready for review. Please let me know if something has to be fixed or improved.

@kiplingw kiplingw merged commit 769c0c3 into pistacheio:master Apr 24, 2024
23 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants