search

naomijub / edn-rs

[DEPRECATED]: Crate to parse and emit EDN
https://crates.io/crates/edn-rs
MIT License
81 stars 14 forks source link

Fuzzing fixes #147

Closed Grinkers closed 8 months ago

Grinkers commented 9 months ago

Cleaned up/rebased version of https://github.com/edn-rs/edn-rs/pull/125.

This PR is mostly for the tests and history, the actual parsing implementation (clone version) will be replaced in a future PR with the EdnError/parse rework as seen in https://github.com/Grinkers/edn-rs/pull/6. I can squash it all, but I think this will make things easier to review.

naomijub commented 9 months ago

Why not check the exact error?

Grinkers commented 9 months ago

Why not check the exact error?

For what?

Generally speaking, this is coming up next. I'm just trying to keep the PR sizes down, but the EdnError rework is pretty big https://github.com/Grinkers/edn-rs/pull/6 (see the "Debug" section) https://github.com/Grinkers/edn-rs/blob/4365f555f997df08ecffce32a65c18b0dd76870d/tests/error_messages.rs https://github.com/Grinkers/edn-rs/blob/4365f555f997df08ecffce32a65c18b0dd76870d/src/edn/error.rs

EdnError won't be string based. It also won't be possible for user/test code to be able to create an arbitrary EdnError. Nor does it have added requirements like Eq or heap allocation.

More testing can be added to the future error_messages.rs. The current tests are useful for finding regressions or parsing issues, but they're not really useful for testing inconsistent/underspecified string-based messages.

After EdnError rework, I also had some WIP instrumentation coverage testing to get 100% coverage for messages, which is where these should be tested/specified https://github.com/Grinkers/edn-rs/commit/eac1f8ba03ba7f095580bb38a8fa757494c510ea#diff-efdbad326cffce82491ae4d979ca521416f195633dca11d3960eefaed5415a0d

Grinkers commented 9 months ago

Rebased and just removed the commit dealing with tests, just to get things untangled.

Grinkers commented 8 months ago

@naomijub I think this is a good time for a 0.17.5. Bonus points if we can do https://github.com/edn-rs/edn-rs/issues/138 at the same time to test it. This fixes a lot of things without introducing breaking changes going forward (I'm assuming the following release will probably be 0.18.x, moving towards a 1.0 goal).