mouse07410 / asn1c

The ASN.1 Compiler
http://lionet.info/asn1c/
BSD 2-Clause "Simplified" License
97 stars 71 forks source link

Output from encoding an OpenType extension is wrong (APER) #143

Closed jet-mark closed 10 months ago

jet-mark commented 11 months ago

When attempting to encode an extension that is an open-type, the output is invalid. No error is raised during encode; but the decode of the buffer fails.

An example of the failed item is the actionDefinition-Format5 item specified by:

   ric-Style-Type             RIC-Style-Type,
   actionDefinition-formats   CHOICE{
      actionDefinition-Format1      E2SM-KPM-ActionDefinition-Format1,
      actionDefinition-Format2      E2SM-KPM-ActionDefinition-Format2,
      actionDefinition-Format3      E2SM-KPM-ActionDefinition-Format3,
      ...,
      actionDefinition-Format4      E2SM-KPM-ActionDefinition-Format4,
      actionDefinition-Format5      E2SM-KPM-ActionDefinition-Format5
   },
   ...
}

Taken from: https://gitlab.eurecom.fr/mosaic5g/flexric/-/blob/master/src/sm/kpm_sm_v2.02/ie/e2sm-kpmv2.02.asn1?ref_type=heads#L207

Checking the encode/decode functionality in constr_CHOICE_aper.c, they were not mirror images of each other - and by making them almost mirror images, it works. I am using these diffs, and it appears to work:

index 0d356f7f..5844968e 100644
--- a/skeletons/constr_CHOICE_aper.c
+++ b/skeletons/constr_CHOICE_aper.c
@@ -168,11 +168,7 @@ CHOICE_encode_aper(const asn_TYPE_descriptor_t *td,
         asn_enc_rval_t rval = {0,0,0};
         if(specs->ext_start == -1)
             ASN__ENCODE_FAILED;
-        int n = present - specs->ext_start;
-        if(n <= 63) {
-            if(n < 0) ASN__ENCODE_FAILED;
-            if(per_put_few_bits(po, n, 7)) ASN__ENCODE_FAILED;
-        } else
+        if(aper_put_nsnnwn(po, ct ? ct->range_bits : 0, present - specs->ext_start))
             ASN__ENCODE_FAILED;
         if(aper_open_type_put(elm->type, elm->encoding_constraints.per_constraints,
                               memb_ptr, po))
mouse07410 commented 11 months ago

@jet-mark did you have a chance to try decoding what's ASN1C encodes with your patch, against NOKALVA OSS playground https://asn1.io/asn1playground/ ? That would be the final validation of your patch, I think...?

jet-mark commented 11 months ago

from my testing, the decode works after encoding (I have a unit test that encodes the structure, decodes the output, and then checks input vs output).

I will check out the playground soon - and update when I have done.

mouse07410 commented 11 months ago

@jet-mark please feel free to add your unit test to this repo as well.

Maybe, you'd like to create a PR?

jet-mark commented 11 months ago

@mouse07410 I notice that you've committed my suggested diffs. Thank you.

I have created a Unit-Test that identifies the original issue (and works with the fix in place) using the test-c-compiler infrastructure already in place (the relevant UT didn't actually do anything previously). Unfortunately, I don't appear to be able to push a local branch, and so cannot create a PR for it.

So, I'm attaching the logs and diffs to this comment in case you want to update the tests. ut_logs.txt choice-59.diffs.txt

(Unfortunately I still haven't managed to check with the playground yet)

mouse07410 commented 10 months ago

@jet-mark I assume all your checks and tests pass now, so closing this issue.

Please feel free to re-open, if you think it's still unresolved.