qnighy / yasna.rs

ASN.1 library for Rust
Apache License 2.0
42 stars 31 forks source link

Fix overflow when reading length, add fuzzer #62

Closed 5225225 closed 2 years ago

5225225 commented 2 years ago

Looks like the project doesn't use rustfmt, is this intentional?

est31 commented 2 years ago

For rustfmt, it still doesn't have the options stabilized/available that I'd like, so atm it's not mature yet I'd say.

5225225 commented 2 years ago

The fuzzer is generated from a template, but you can't autogenerate the whole thing.

decode_ber was hand written as a fairly simple test, and probably could be auto generated, but having it actually in git encourages people to actually run it and report issues, even if it's not actually continuously ran anywhere.

Though in practice, yeah, you're not going to be changing the fuzzer's Cargo.toml all that much, except maybe to change what features it uses from the library.

Personally I don't think it's a great idea to have the fuzzer crate actually as part of the main crate, especially since that would be introducing a workspace where there wasn't one before. I've not done that before, and it doesn't really seem like an intended use-case for cargo-fuzz.

It would be reasonable to make it built in CI, though.

est31 commented 2 years ago

Personally I don't think it's a great idea to have the fuzzer crate actually as part of the main crate

That's not my suggestion. My suggestion is to have a workspace with two crates. Then the Cargo.lock is shared as well as the target dir. Only one directory to clean.

est31 commented 2 years ago

The fuzzer is generated from a template, but you can't autogenerate the whole thing.

I see. It can stay then. :+1: