feat(cddl): add .cbor and .cborseq control operators (#89) - #115
Merged
Merged
Conversation
|
RFC 8610 §3.8.4: a .cbor control on a byte string asserts the byte string carries a CBOR-encoded data item matching the given type; .cborseq asserts it carries a sequence of CBOR items matching the given (array) type. This is exactly how COSE (RFC 9052) describes a COSE_Sign1's protected header or payload field. Verified the semantics directly against the RFC text before porting, not just the PR description. Parser (packages/cddl): no new parsing logic needed - the operator's value is a plain type reference, which parseOperator already handles generically for every other operator (bits, and, within, eq, ne, lt, le, gt, ge). Just the two OPERATORS/OPERATORS_EXPECTING_VALUES table entries plus the OperatorType union member. Also updated docs: examples/commons/operators.cddl (the shared "one example per operator" fixture already covered every other operator, so .cbor/.cborseq were a visible gap by omission) and packages/cddl/docs/operators.md, whose hardcoded OperatorType union, numbered operator list, and worked per-operator examples were all stale in the exact same way - checked every doc file that enumerates operators, not just the obvious one. Ported from Mearman#1 (stacked on webdriverio#88, already merged upstream, so applies directly to main). One correction made along the way: that PR's .cbor test asserted Occurrence: { n: 0, m: Infinity } for a `?`-marked member, but this repo's parser correctly computes { n: 0, m: 1 } for `?` (zero-or-one, matching every other optional-member test in this file) - m: Infinity is `*`, not `?`. Fixed the ported test's expectation rather than carry over what looks like a fork-specific parser divergence or copy-paste error. Also added a .cborseq parser test - the ported PR only covered .cbor. Generators: checked all five, not just cddl. cddl2ts/cddl2py/ cddl2swift/cddl2kotlin already ignore any operator they don't specifically special-case (only "default" is ever checked), so .cbor/ .cborseq fall through generically to the base bstr type - verified via their actual transform() output, not assumed, and added a test to each. cddl2java was the exception, and needed real code changes: its parseType() throws "Unknown operator" for any operator whose base type isn't bool/range/group - .cbor/.cborseq (attached to plain bstr) hit that path and crashed. Digging into why also surfaced a second, pre-existing bug: cddl2java had no bstr/bytes -> byte[] mapping at all (bare `bstr` alone resolved to "Unknown"), unlike every other generator. Latent until now since bstr never appears in the real webdriver-bidi spec cddl2java is tested against - but .cbor/.cborseq are RFC-defined to apply specifically to byte strings, so properly supporting this port in cddl2java requires both fixes, not just catching the crash. Fixed both: added the missing byte[] mapping, and made the operator-fallthrough resolve the base type instead of throwing for any operator it doesn't specifically recognize (the same generic behavior the other four generators already have). Deliberately did not add .cbor/.cborseq to examples/commons/test.cddl, the realistic BiDi-shaped fixture 4 of the 5 generators snapshot-test against - unlike operators.cddl (a catalog, one example per feature), test.cddl reads as an actual mini protocol spec, and a CBOR/COSE property doesn't fit that narrative. Touching it would also mean regenerating snapshots across 4 packages for coverage the explicit per-generator tests above already give more precisely. Full checks:all passes (464 tests, coverage thresholds met). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
RFC 8610 §3.8.4: .cborseq matches the decoded sequence, taken as an array, against its controller type - unlike .cbor, which matches a single decoded item and can legitimately be a map. Both examples/commons/operators.cddl and the .cborseq parser test reused the same map-typed coseHeader/inner rule .cbor already (correctly) used, silently modeling invalid CDDL. Fixed by giving .cborseq its own array-typed controller: coseHeaders = [coseHeader] in the fixture, innerList = [int] in the parser test. Verified both corrected AST shapes by actually running the parser before writing the assertions, not just adjusting the type name. operators.md's prose already said "array type" without repeating the invalid example, so it didn't need a change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…generators Not a functional bug - none of these tests ever define a concrete shape for the referenced type, so the RFC 8610 §3.8.4 map-vs-array distinction fixed at the parser level (1e16479) couldn't actually be violated here; every generator ignores the operator's value entirely regardless. But all five reused the same bare "inner" name for both .cbor and .cborseq, which reads as an inconsistency right next to the parser-level fix distinguishing coseHeader (map) from coseHeaders/innerList (array). Split each shared it.each into [operator, referencedType] pairs so the naming matches what the RFC actually requires, even though it makes no difference to what's being tested. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dprevost-LMI
force-pushed
the
port-cbor-operators
branch
from
September 22, 2026 23:58
c40b96d to
2409fd7
Compare
Collaborator
Author
|
@greptileai, can you review again? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #89
RFC 8610 §3.8.4: a .cbor control on a byte string asserts the byte string carries a CBOR-encoded data item matching the given type; .cborseq asserts it carries a sequence of CBOR items matching the given (array) type. This is exactly how COSE (RFC 9052) describes a COSE_Sign1's protected header or payload field. Verified the semantics directly against the RFC text before porting, not just the PR description.
Parser (packages/cddl): no new parsing logic needed - the operator's value is a plain type reference, which parseOperator already handles generically for every other operator (bits, and, within, eq, ne, lt, le, gt, ge). Just the two OPERATORS/OPERATORS_EXPECTING_VALUES table entries plus the OperatorType union member. Also updated docs: examples/commons/operators.cddl (the shared "one example per operator" fixture already covered every other operator, so .cbor/.cborseq were a visible gap by omission) and packages/cddl/docs/operators.md, whose hardcoded OperatorType union, numbered operator list, and worked per-operator examples were all stale in the exact same way - checked every doc file that enumerates operators, not just the obvious one.
Ported from Mearman#1 (stacked on #88, already merged upstream, so applies directly to main). One correction made along the way: that PR's .cbor test asserted Occurrence: { n: 0, m: Infinity } for a
?-marked member, but this repo's parser correctly computes { n: 0, m: 1 } for?(zero-or-one, matching every other optional-member test in this file) - m: Infinity is*, not?. Fixed the ported test's expectation rather than carry over what looks like a fork-specific parser divergence or copy-paste error. Also added a .cborseq parser test - the ported PR only covered .cbor.Generators: checked all five, not just cddl. cddl2ts/cddl2py/ cddl2swift/cddl2kotlin already ignore any operator they don't specifically special-case (only "default" is ever checked), so .cbor/ .cborseq fall through generically to the base bstr type - verified via their actual transform() output, not assumed, and added a test to each.
cddl2java was the exception, and needed real code changes: its parseType() throws "Unknown operator" for any operator whose base type isn't bool/range/group - .cbor/.cborseq (attached to plain bstr) hit that path and crashed. Digging into why also surfaced a second, pre-existing bug: cddl2java had no bstr/bytes -> byte[] mapping at all (bare
bstralone resolved to "Unknown"), unlike every other generator. Latent until now since bstr never appears in the real webdriver-bidi spec cddl2java is tested against - but .cbor/.cborseq are RFC-defined to apply specifically to byte strings, so properly supporting this port in cddl2java requires both fixes, not just catching the crash. Fixed both: added the missing byte[] mapping, and made the operator-fallthrough resolve the base type instead of throwing for any operator it doesn't specifically recognize (the same generic behavior the other four generators already have).Deliberately did not add .cbor/.cborseq to examples/commons/test.cddl, the realistic BiDi-shaped fixture 4 of the 5 generators snapshot-test against - unlike operators.cddl (a catalog, one example per feature), test.cddl reads as an actual mini protocol spec, and a CBOR/COSE property doesn't fit that narrative. Touching it would also mean regenerating snapshots across 4 packages for coverage the explicit per-generator tests above already give more precisely.
Full checks:all passes (464 tests, coverage thresholds met).