tdewolff / parse

Go parsers for web formats
MIT License
408 stars 64 forks source link

Fuzzing for stable stringification #109

Closed Adjective-Object closed 10 months ago

Adjective-Object commented 1 year ago

Adds fuzzing tests for Parse / .JS() to validate the following:

Fuzz saves these testcases to the testdata/fuzzfolder and adds them to the normal test run.

To discover more failing cases, run

As part of this I fix, I also address some idempotency issues in existing test-suites and things I found with some short fuzzing:

Note that this is probably not the end of fuzzing errors. I've only been able to fuzz for up to 5 minutes at a time before finding a new failure.

Related to the discussion in #108

tdewolff commented 10 months ago

I'm very sorry for the extremely late response. This PR had completely slipped the radar. Thank you very much for this contribution, it contains some valuable bug fixes. Fuzzing going back and forth with JS() is a good idea.

Instead of merging, I've completely restructured the ast.go code base (reducing code duplication and superfluous error checking). See 38da6d49822b37d16bc1ab1caa08a168db3c0e55. You should use ast.JSString() to get a string, or otherwise use the JS(io.Writer) interface on all nodes. Similarly for JSON.

I've also added the fuzzing script together with the other fuzzers in tests/ so that they are run automatically with OSSFuzz periodically. Your bug fixes have been incorporated with the following commits: bf2964b45b940153f1cc46df11da34889da7c7e6 9b8eaa4f0272c227e3edfa6fef37f8515a92cf71

I believe the changes in this PR are thus incorporated and I'll close this PR. I'll keep adding fixes for crashes found with fuzzing, thanks again for the effort!!

tdewolff commented 9 months ago

Just wanted to follow up that your idea was a tremendous source for discovering bugs in the JS output and in the JS parser itself! I get at least 2 new bug each week, thanks again!