APER encode-decode round trip fails where other codecs succeed

[jkalez]

It appears that in the following example, APER codec fails needing more bits where other codecs succeed:

use rasn::AsnType;

#[derive(rasn::AsnType, rasn::Encode, rasn::Decode, Debug, Clone, PartialEq, Eq)]
#[rasn(automatic_tags, option_type(Option))]
#[non_exhaustive]
pub struct Updates {
    pub updates: Vec<u8>,
}

#[derive(rasn::AsnType, rasn::Encode, rasn::Decode, Debug, Clone, PartialEq, Eq)]
#[rasn(automatic_tags, option_type(Option))]
#[rasn(choice)]
#[non_exhaustive]
pub enum Message {
    Updates(Updates),
}

fn main() {
    let msg = Message::Updates(Updates { updates: vec![0] });
    let buf = rasn::ber::encode(&msg).unwrap();
    let deser: Message = rasn::ber::decode(&buf).unwrap();
    assert_eq!(deser, msg);
    let buf = rasn::cer::encode(&msg).unwrap();
    let deser: Message = rasn::cer::decode(&buf).unwrap();
    assert_eq!(deser, msg);
    let buf = rasn::der::encode(&msg).unwrap();
    let deser: Message = rasn::der::decode(&buf).unwrap();
    assert_eq!(deser, msg);
    let buf = rasn::uper::encode(&msg).unwrap();
    let deser: Message = rasn::uper::decode(&buf).unwrap();
    assert_eq!(deser, msg);
    let buf = rasn::aper::encode(&msg).unwrap();
    let deser: Message = rasn::aper::decode(&buf).unwrap();
    assert_eq!(deser, msg);
}

When I run this example, I get the following error:

thread 'main' panicked at src/main.rs:33:51:
called `Result::unwrap()` on an `Err` value: DecodeError { kind: FieldError { name: "Updates.updates", msg: "Error Kind: Need more BITS to continue: (Size(8))....

Then a lot of snafu backtrace stuff

I believe that I have set my types up correctly and APER should work here, as all of the other codecs do, but there's a chance I'm just using the API incorrectly? If that's the case, I'd argue the error messages could be clearer here

[XAMPPRocky]

Thank you for your issue! I don't have the time to investigate further, but I'd be happy to review the fix.

The best way to debug this is to sprinkle some dbg! statements for the input, before it decodes the sequence of, and before it parses the integer, that should tell us if it's encoded incorrectly or if the parser is at fault.

My initial guess is the encoder maybe not respecting the constraints of the type inside the sequence of, but that's just a guess.

[Nicceboy]

Issue is on #[non_exhaustive], when you remove them, it works normally. There is something wrong on handling the extension additions with APER.

Technically pub updates: Vec<u8>, should be same as pub updates: OctetString. They seem to fail similarly, but they are running different parts of the code.

Is Vec<u8> expected to represent u8 constrained SequenceOf <Integer>?

FieldError might need some improvements; it seems to always be wrapper of inner errors when there is error in Sequence and currently inner error is just as string format, which includes unformatted backtrace of inner error.

[XAMPPRocky]

Is Vec expected to represent u8 constrained SequenceOf ?

If you're asking me, yes that is the intention.

Yes, field error could be a lot better I hope that the #187 would help make it easier to pass context of identifiers.

[Nicceboy]

This seems rather difficult to solve, if I understand correctly, and I don't have time to do more right now.

personnel.rs is missing test for ExtensiblePersonnelRecord for APER (Standard, section A.3.3 ALIGNED PER representation of this record value). Making this test would be helpful on debugging what is actually wrong with correct values.

It seems that padding should be calculated differently, and with current implementation it is very difficult. Based on the standard, extension field bitmap values / preamble should be noted when calculating the alignment/padding of the length encoding of the first field of the set/sequence.

Currently encoding of the fields are calculated first before actual extension bitfield values are calculated, and padding is added between length and value without noting the preamble.

[XAMPPRocky]

Currently encoding of the fields are calculated first before actual extension bitfield values are calculated, and padding is added between length and value without noting the preamble.

Correct me if I'm wrong, but I think is solved by updating the output_length function, which does look at what the value should be with padding and is what is used for pad_to_alignment, so as long as output_length matches what it should, and we call pad_to_alignment in the correct place, where the encoding of fields happens shouldn't matter.

https://github.com/XAMPPRocky/rasn/blob/e6ad63480f7396e9aa9d9c11690e26796ed78de1/src/per/enc.rs#L125-L144

[Nicceboy]

If there are many nested sequences, somehow we should be able to get the the preamble information for each of them, up to the root, and note it on the first time when APER attempts to pad to alignment.

We should also know when constructing set/sequence whether it is extensible, while not having any extensible fields, and also all the optional/default fields, up to the root.

I could not figure out yet how to get the extensibility information on runtime if the sequence does not have any extensible fields but has the marker. Maybe there is some easy way?

[XAMPPRocky]

if the sequence does not have any extensible fields but has the marker. Maybe there is some easy way?

Shouldn't you be able to do that by checking if T::EXTENSIBLE_FIELDS.is_some()? It should be some with an empty slice.

[Nicceboy]

Shouldn't you be able to do that by checking if T::EXTENSIBLE_FIELDS.is_some()? It should be some with an empty slice.

Thanks, that helped. I just did not understood the code...

I was able to fix the problems related to standard test case (ExtensiblePersonnelRecord completes now), but this original issue might need skills I don't have.

Is there were to get choice extensible status as well? If there is, then also this can be fixed, but understanding the macros might take too much my time if it needs to be added.

[Nicceboy]

Actually figured it out, it was E::CONSTRAINTS.extensible(). PR can be reviewed.

[XAMPPRocky]

Is there were to get choice extensible status as well?

It's similar T::EXTENDED_VARIANTS.is_some()

[Nicceboy]

It's similar T::EXTENDED_VARIANTS.is_some()

I thought it was only for enumerated... well I guess the current solution also works. This issue can be closed now I think.