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

Fuzzing improvements #201

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Conversation

tyler92
Copy link
Contributor

@tyler92 tyler92 commented Oct 20, 2024

Major improvements:

  • the fuzzer performs not only schema parsing but validation too (the most interesting part of the library)
  • the fuzzer performs parsing in-memory avoiding temporary files which improves fuzzing speed
  • seed corpus is based on JSON-Schema-Test-Suite/tests files

Minor improvements:

  • avoid return 1 from fuzz target in case of exceptions - it makes fuzzing slower
  • the line sed -i '27d' include/valijson/utils/rapidjson_utils.hpp was removed from tests/fuzzing/oss-fuzz-build.sh. A proper change is applied to the fuzzing test instead
  • compile fuzzer with -Dvalijson_BUILD_TESTS=FALSE (there is no sense in compiling unit tests for fuzzing)
  • added -j argument to zip to build a flat list of files (subdirectories are not consumed by the fuzzer)

List of issues found by new approach: #197 #198 #199 #200

@tristanpenman
Copy link
Owner

Thanks @tyler92, this is really interesting work! I appreciate you taking the time to raise PRs, and help improve the library.

It might take a little while for me to merge everything. I tread pretty carefully these days, as Valijson is closer to being in maintenance mode than active development. That said, I'll try to set aside some time to do so in the next week, so that we can get these fixes into a proper release.

@tristanpenman tristanpenman merged commit bd1f707 into tristanpenman:master Oct 22, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants