der: Support custom error types.

[turbocool3r]

I'm currently developing a library for parsing the Image4 format found in boot images on Apple devices which is based on DER. This format has some constraints not related to the DER and I'd like to provide a custom error type that implements From<der::Error> and has some additional variants I need. Is this something that would be possible to add?

I think the best way to do it is to add associated Error types in the Decode and Encode traits and wherever needed. I'll be happy to provide a PR.

Here is an example of a file I want to parse if you'd like to understand my case better. This recursive structure also creates compile-time recursion when used with the read_nested method btw. Should I file an issue?

[tarcieri]

Seems like you already filed an issue 😉

Yeah, we do something like this in the ssh-encoding crate: https://docs.rs/ssh-encoding/latest/ssh_encoding/trait.Decode.html

I'm generally supportive of it, although it'd be a pretty big change in terms of overall code churn. Probably worth it, though.

[turbocool3r]

Nice! I'll try to make a PR when I'll have some time.

[turbocool3r]

@tarcieri I've tried implementing this, and there are some problems when it comes to DerOrd: implementing DerOrd will typically require constructing a header which needs a call either to encoded_len or value_len (see this). Due to DerOrd being used in some ValueOrd implementations this also affects the latter trait. I see some solutions here:

1. Make DerOrd and ValueOrd depend on Encode, Decode or both of them. This will potentially force trait users to implement unnecessary traits.
2. Add associated Error types to both DerOrd and ValueOrd. This will complicate where clauses when these traits are used together or with Encode/Decode traits.
3. A common base trait that provides the error type. This might reduce flexibility and increase boilerplate.

Another problem is that SliceReader updates the positions in its decode method, which is strange since you construct your error from the decoder anyway. This requires some sort of error trait or a little redesign of the way errors are handled.

[tarcieri]

I don't understand what the problem is. Can you explain which error types are incompatible in this case?

Did you use the same approach as ssh-encoding where the associated error type is bounded on From<der::Error>?

[turbocool3r]

Yes, the approach is the same. The problem is when you set a trait bound like DerOrd + Decode<'a> there are two Error types that are not necessarily compatible. If you write something like:

fn foo<'a, T: DerOrd + Decode<'a>>(bytes: &'a [u8], bar: &T) -> Result<T, <T as Decode>::Error> {
    let baz = T::from_der(bytes)?;
    let a = baz.der_cmp(bar)?;
    ...
    Ok(baz)
}

there will be a compiler error because <T as Decode>::Error might not be constructible from <T as DerOrd>::Error.

[tarcieri]

So, this line?

 let a = baz.der_cmp(bar)?;

But if there's a From<der::Error> bound on <T as Decode>::Error, that should take care of it.

[turbocool3r]

It isn't really possible to not add an associated Error type to DerOrd as well as the Encode/Decode traits if we want to use the latter traits while implementing DerOrd. Here is a good example of why der_cmp can't return der::Error in that case.

[tarcieri]

How about only parameterizing the type for Decode/DecodeValue?

[turbocool3r]

That is a good idea.

[dishmaker]

I've done custom error handling for Decode / DecodeValue.
It works perfectly but I had to use std::result::Result everywhere.

Also renamed Error to DerError.

/// Reader trait which reads DER-encoded input.
pub trait Reader<'r>: Sized {
    /// Error returned by the reader
    type ERR: From<DerError> + From<ErrorKind> + DerDebugErr;

    fn read_nested<'n, T, F>(&'n mut self, len: Length, f: F) -> std::result::Result<T, Self::ERR>
    where
        F: FnOnce(&mut NestedReader<'n, Self>) -> std::result::Result<T, Self::ERR>,
    {
        let mut reader = NestedReader::new(self, len)?;
        let ret = f(&mut reader).wrap_err_nested::<T>()?;
        reader
            .finish(ret)
            .wrap_err_field("finish()")
            .wrap_err_nested::<T>()
    }

Note: wrap_err_nested and wrap_err_field are custom generic fns that do nothing till overriden for ERR type.
So these functions are zero-cost (they optimize out at compile-time).

/// Used to pretty-print errors
pub trait DerDebugErr {
    /// Called when reading a nested sequence
    fn wrap_err_nested<X>(self) -> Self;

    /// Called when reading a field
    /// 
    /// auto-generated calls in `#[derive(Sequence)]`
    fn wrap_err_field(self, field_name: &str) -> Self;

    /// Called when reading an entry in Vec sequence/set
    fn wrap_err_index(self, index: usize) -> Self;

[tarcieri]

Fixed in #1055

[dishmaker]

Omg why are these generic Errors on a Decode trait :( 😢 😭

It would be a lot better on Reader trait.

/// Reader trait which reads DER-encoded input.
pub trait Reader<'r>: Sized {
    /// Error returned by the reader
    type ERR: From<DerError> + From<ErrorKind>;

I can't throw my error from a sequence for example 😭 😭

impl<'a> DecodeValue<'a> for SequenceRef<'a> {
    type Error = Error; // <-- here it's fixed to der::Error

... or a #[derive(Sequence)] 😭

impl #impl_generics ::der::DecodeValue<#lifetime> for #ident #ty_generics #where_clause {
    type Error = ::der::Error;

[tarcieri]

@dishmaker I already asked you once to act like an adult when posting to this issue tracker. We don't have a formal CoC, but please don't make me ask you again or I may have to take action including limiting your ability to interact with this project. Posts filled with crying face emojis are not appropriate here.

Omg why are these generic Errors on a Decode trait

To enable format-specific custom error types, exactly as stated in the description.

It would be a lot better on Reader trait.

That's something we can consider separately (and also a false dichotomy). Since we only have two reader types and their error cases are all covered it's not a terribly pressing issue IMO.

Note that custom error types on the Reader trait don't solve the OP's problem, which is errors which are specific to a particular encoding format.

... or a #[derive(Sequence)]

A way to override the error type when using custom derive is an important missing feature that should be easy to add.