Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested choices not completly fixed on PER #168

Closed
Nicceboy opened this issue Oct 6, 2023 · 2 comments · Fixed by #169
Closed

Nested choices not completly fixed on PER #168

Nicceboy opened this issue Oct 6, 2023 · 2 comments · Fixed by #169

Comments

@Nicceboy
Copy link
Contributor

Nicceboy commented Oct 6, 2023

Seems like the issue of infinite recursion was not completely fixed in #148 and #157 .

The fix only works if some parent type is struct or set.
In nested choices, the problem still occurs.

I tried fixing it by modifying the function encode_explicit_prefix to following:

    fn encode_explicit_prefix<V: Encode>(
        &mut self,
        tag: Tag,
        value: &V,
    ) -> Result<Self::Ok, Self::Error> {
        if let Some((_, true)) = self.field_bitfield.get(&tag) {
            value.encode(self)
        } else if self.field_bitfield.get(&tag).is_none() {
            // There is no bitfield if none of the parent objects is struct/set
            // But we still need to handle nested choices explicitly
            value.encode(self)
        } else {
            self.set_bit(tag, true)?;
            value.encode_with_tag(self, tag)
        }
    }

In OER it works, but in PER it works only for two levels, and on third level the values are halved for some reason?

asn1tools example:

import asn1tools


def main():
    # Compile the ASN.1 module
    spec = asn1tools.compile_string(
        """MyModule DEFINITIONS AUTOMATIC TAGS ::= BEGIN

       Choice ::= CHOICE {
           normal INTEGER,
           high   INTEGER,
           medium INTEGER
       }
       BoolChoice ::= CHOICE {
           a BOOLEAN,
           b BOOLEAN,
           c Choice
       }
       TripleChoice ::= CHOICE {
           a BOOLEAN,
           b BoolChoice
       }
       END""",
        codec="uper",
    )

    # Encode a value
    # value = {'c': {'normal': 333}}
    value = ("b", ("c", ("normal", 333)))
    encoded = spec.encode("TripleChoice", value)

    # Print the encoded value
    for e in encoded:
        print(f"{e}, ", end="")


if __name__ == "__main__":
    main()

Will produce 192, 16, 10, 10.

On rasn encoding, the output is exactly half:

thread 'uper::tests::only_nested_choice' panicked at 'assertion failed: `(left == right)`
Diff < left / right > :
 [
<    192,
<    16,
<    10,
<    104,
>    96,
>    8,
>    5,
>    52,
 ]

If I add one nesting level for asn1tools, then it produces the same output.
Adding one nesting level more to rasn, it changes output to even smaller value (quarter of previous)

Any ideas if there is some optimisation in PER which should not happen here?

@Nicceboy
Copy link
Contributor Author

Nicceboy commented Oct 6, 2023

asn1rs produces the same output than asn1tools. Maybe that is the correct one.

[src/lib.rs:36] &writer.into_bytes_vec() = [
    192,
    16,
    10,
    104,
]

@Nicceboy
Copy link
Contributor Author

Nicceboy commented Oct 6, 2023

After many hours of debugging, the problem seems to be on the invalid range constraint when encoding the integer of Choice selection.

When there are only two options on Choice then both options can be described with single bit (either 0 or 1)

Minimum for range constraint was 3 so it always used 2 bits at minimum to encode the selection.

@Nicceboy Nicceboy changed the title Nested choices not completly fixed Nested choices not completly fixed on PER Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant