Decoding of set_of and sequence_of suppressing decode error in child elements?

[Arnix]

Hi

Again, thanks for the hard work. A little observation while trying to get closer to something that be be brought to production

Problem statement

I have come across scenarios where mysteriously I ended up with empty "collections". To make it presentable I made a small test, that is enclosed at the bottom.

Basically I think this comes down to how set_of and sequence_of are populated (example from bar/de.rs) :

fn decode_sequence_of<D: Decode>(
        &mut self,
        tag: Tag,
        _: Constraints,
    ) -> Result<Vec<D>, Self::Error> {
        self.parse_constructed_contents(tag, true, |decoder| {
            let mut items = Vec::new();
            while let Ok(item) = D::decode(decoder) {
                items.push(item);
            }
            Ok(items)
        })
    }

Any failure of a child (or grandchild) element in this sequence_of will be hidden as "no more elements" and taken as the normal flow.

Suggested work around

Unless I am totally missing out on something here, I would suggest to do something like this:

    fn decode_sequence_of<D: Decode>(
        &mut self,
        tag: Tag,
        _: Constraints,
    ) -> Result<Vec<D>, Self::Error> {
        self.parse_constructed_contents(tag, true, |decoder| {
            let mut items = Vec::new();
            loop {
                match D::decode(decoder) {
                    Ok(item) => items.push(item),
                    Err(err) => {
                        return match *err.kind {
                            DecodeErrorKind::Incomplete { .. } => Ok(items),
                            _ => Err(err),
                        }
                    }
                }
            }
        })
    }

I am more than happy to submit a PR if my findings are correct, otherwise I will learn something new today :-)

The test code to exercise the behaviours:

#[cfg(test)]
mod test {
    use rasn_derive::{AsnType, Decode, Encode};

    #[derive(Debug, AsnType, Encode, Decode, Copy, Clone, PartialEq)]
    #[rasn(enumerated)]
    pub enum Bar {
        Deliver = 0,
        SubmitReport = 1,
        StatusReport = 2,
        DeliverReport = 4,
        Submit = 5,
        Command = 6,
    }

    #[derive(Debug, AsnType, Encode, Decode)]
    #[rasn(automatic_tags)]
    pub struct FooWithBar {
        bar : Bar,
        bars : Vec<Bar>,
    }


    #[test]
    fn test_foo_with_bar() {
        let foo_w_b = FooWithBar {
            bar : Bar::Deliver,
            bars : vec![Bar::DeliverReport, Bar::Submit]
        };

        let encoded = rasn::der::encode(&foo_w_b).unwrap();
        // println!("{:?}", encoded);

        // set bar to legal value SubmitReport
        let mut tweaked = encoded.clone();
        tweaked[4] = 1;
        match rasn::der::decode::<FooWithBar>(tweaked.as_slice()) {
            Ok(decoded) => {
                println!("{:?}", decoded);
            }
            Err(_) => {
                panic!()
            }
        }

        // set bar to illegal value > Command
        let mut tweaked = encoded.clone();
        tweaked[4] = 7;
        match  rasn::der::decode::<FooWithBar>(tweaked.as_slice()) {
            Ok(decoded) => {
                println!("{:?}", decoded);
                panic!()
            }
            Err(_err) => {
                // dbg!(_err);
            }
        }

        // set bar to legal value Command
        let mut tweaked = encoded.clone();
        tweaked[12] = 6;
        match rasn::der::decode::<FooWithBar>(tweaked.as_slice()) {
            Ok(decoded) => {
                // println!("{:?}", decoded);
            }
            Err(_) => {
                panic!()
            }
        }

        // set bar to illegal value > Command
        let mut tweaked = encoded.clone();
        tweaked[12] = 7;
        match rasn::der::decode::<FooWithBar>(tweaked.as_slice()) {
            Ok(decoded) => {
                println!("{:?}", decoded);
                panic!()
            }
            Err(_err) => {
                dbg!(_err);
            }
        }

    }
}

[XAMPPRocky]

I am more than happy to submit a PR if my findings are correct, otherwise I will learn something new today

Thank you for your question, your findings are correct, however I think I want to have a different solution. Instead of the current loop situation, I think what I would prefer the solution to be is that we return the Option<usize> inside the closure, representing the length if present, and then the implementations can decode a explicit number of times, or continously until it hits the length marker. This would be both more performant, and allow us to support types like fixed arrays.

[Arnix]

Thanks for the feedback!