cddl icon indicating copy to clipboard operation
cddl copied to clipboard

Fix group and type choice parsing and validation errors

Open anweiss opened this issue 2 years ago • 8 comments

Fixes #116 and #120.

anweiss avatar Apr 19 '22 18:04 anweiss

@tomachristian feel free to give this a sanity check with some of your scenarios. Will get a separate PR up for smoke tests.

anweiss avatar Apr 19 '22 18:04 anweiss

Hello @anweiss.

Thank you for this fix, it works to fix specifically that scenario, but I found one where it does not work

tester = [$$val]
$$val //= (
  type: 10,
  data: uint
)
$$val //= (
  type: 11,
  data: tstr
)

with CBOR

82           # array(2)
   0B        # unsigned(11)
   63        # text(3)
      537472 # "Str"

it fails with

  [ERROR] Validation of "/repo/.nuke/temp/ca2adc50-ef43-4301-aee2-e1ff1e0f8eb3.cbor" failed: error validating group choice group entry associated with rule "val" at cbor location : expecting array with length 1, got 2
  error validating group choice group entry associated with rule "val" at cbor location : expecting array with length 1, got 2
  error validating group choice group entry associated with rule "val" at cbor location : expecting array with length 1, got 2
  error validating group choice group entry associated with rule "val" at cbor location : expecting array with length 1, got 2
  error validating group choice group entry associated with rule "val" at cbor location : expecting array with length 1, got 2

Thank you!

tomachristian avatar Apr 20 '22 07:04 tomachristian

Thanks @tomachristian! Keep 'em coming! I believe I've fixed this with the most recent commit, but feel free to give it a whirl.

anweiss avatar Apr 20 '22 16:04 anweiss

Scratch that ... need to fix more of the logic here.

anweiss avatar Apr 20 '22 16:04 anweiss

@tomachristian I think I've got this fixed now

anweiss avatar Apr 21 '22 17:04 anweiss

Thank you, @anweiss!

This specific scenario works, but found this:

tester = [$$val]
$$val //= (
  type: 10,
  something: uint
)

works with input

82    # array(2)
   0A # unsigned(10)
   01 # unsigned(1)

but, this

tester = [$$val]
$$val //= (
  type: 10,
  extra
)
extra = (
  something: uint
)

does NOT work with the same input.

Also, it seems that validation of float ranges requires the input to be a float, it does not work with ints event though ints are floats.

tester = -273.15..220.9

is ok with F9 4900 # primitive(18688), but not with 0A # unsigned(10).

PS: Sorry I placed everything in here, we can split them in separate issues if you want, I don't mind.

Thank you, again!

tomachristian avatar Apr 26 '22 08:04 tomachristian

Thanks @tomachristian. Working through some more fixes.

anweiss avatar May 13 '22 14:05 anweiss

hey @tomachristian, pushed up a couple of tweaks. Not 100% sure if I've fixed the issue, but feel free to throw some more examples at it and let me know if something still isn't working.

Also re. range validation, I interpreted the spec quite literally ... per https://datatracker.ietf.org/doc/html/rfc8610#section-2.2.2.1:

CDDL currently only allows ranges between integers (matching integer values) or between floating-point values (matching floating-point values).

Thus, if it is a floating point range, validation should only succeed against a float value that has not been coerced from an integer. CC @cabo for thoughts. Happy to move this to a Discussion as needed.

anweiss avatar Jun 27 '22 15:06 anweiss