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

Fuzz experimental parser #3806

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

gguillemas
Copy link
Contributor

@gguillemas gguillemas commented Apr 4, 2024

Thank you for submitting this pull request! We really appreciate you spending the time to work on these changes.

What is the motivation?

To start fuzzing the new parser to identify any issues with it before it becomes the default. The old parser has been fuzzed in OSS-Fuzz for a year and no new issues have been identified for months. Since no significant changes are planned on the old parser, it makes sense to start fuzzing the new one in OSS-Fuzz instead. Fuzzing the old parser is still possible in branch 1.x.

What does this change do?

Enables the parser2 feature in the fuzzing build so that the new experimental parser is used. Enforces that the fuzzer check passes and updates the README file with current information. The fuzzer check will fail until google/oss-fuzz#11777 is merged. After the new parser is the default in main, the parser2 feature can be dropped from Cargo.toml.

What is your testing strategy?

Tested the build locally on the google/oss-fuzz#11777 branch. Ensure that the fuzzer check passes in CI.

Is this related to any issues?

No.

Does this change need documentation?

  • No documentation needed

Have you read the Contributing Guidelines?

@rushmorem
Copy link
Collaborator

parser2 is actually becoming the default and only parser on main very soon.

@gguillemas
Copy link
Contributor Author

parser2 is actually becoming the default and only parser on main very soon.

Thank you! I was aware after our conversation, I just wanted to have a branch where I was able to start testing. If the change happens before google/oss-fuzz#11777 is merged, I will drop the parser2 before marking this PR as ready. Otherwise, it would be nice to start fuzzing ASAP. I am happy to make a separate PR once parser2 is the default on main.

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.

None yet

2 participants