RustCrypto / formats

Cryptography-related format encoders/decoders: DER, PEM, PKCS, PKIX
229 stars 122 forks source link

der: add `Error::(set_)source`; remove `Clone` + `Copy` #1328

Closed tarcieri closed 4 months ago

tarcieri commented 6 months ago

When the std feature is enabled, adds support for propagating an arbitrary error source which can be set using Error::set_source.

The main use case is passing back an arbitrary error value in the ErrorKind::Value use case, to handle passing back what specifically was wrong with a given value in a DER-encoded document.

This entails removing the Clone + Copy bounds since the error is stored in a Box, which can't be Copy and can't reflect a Clone bound. Fortunately, nothing in this repo ever seems to clone an Error so it's not really an issue.

Also adds Error::source impls to the downstream error types in pkcs1, pkcs5, pkcs8, sec1, and spki.

turbocool3r commented 6 months ago

I like this approach, but I still think this serde-like way I suggested in #1055 might be the way to go if there is a need to make error processing zero-cost.

One might use a pattern when a few decoders are tested in sequence and the process is only continued when certain internal errors occur (similar to trying to decode something as different DER types) which might be less efficient with this dynamic approach. Dependency on std also prevents some use cases like this crate of mine which supports no_std (for reasons) and would benefit from custom errors. Personally I think if there might be a breaking change it's worth doing one which allows to use more patterns.

I can update the PR if you think it's worth seeing how well the other approach works with the current code.