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

Fix AuthenticationExtensionsAuthenticatorInputs/Outputs CDDL #2219

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tidoust
Copy link
Member

@tidoust tidoust commented Dec 3, 2024

According to the CDDL grammar, after a control operator (called ctlop in the grammar), there can only be a type2 production. In a type2 production, wrapping parentheses can only be used to wrap a type production. tstr => any is a group production. As such, unless I missed something, it cannot be wrapped in parentheses to produce a type2 production. It rather needs to be wrapped in curly braces or brackets.

In other words, from a CDDL grammar perspective, this is an invalid type:

foo .within ( tstr => any )

This is a valid type:

foo .within { tstr => any }

This update fixes the CDDL type definitions that used the .within operator with an invalid type2.

I don't believe the change introduces any semantic change (so more an editorial update than anything else) but note I'm no CDDL expert, I just happened to have been playing with the CDDL grammar recently... If the semantics change, well, the problem still stands: the current definition is not valid CDDL.


Preview | Diff

According to the CDDL grammar, after a control operator (called `ctlop` in the
ABNF grammar), there can only be a `type2` production:
https://datatracker.ietf.org/doc/html/rfc8610#appendix-B

In a `type2` production, wrapping parentheses can only be used to wrap a `type`
production. `tstr => any` is a `group` production, and needs to be wrapped in
curly braces or brackets.

In other words, from a CDDL grammar perspective, this is an invalid type:
  `foo .within ( tstr => any )`

This is valid:
  `foo .within { tstr => any }`

This update fixes the CDDL type definitions that used the `.within` operator
with an invalid type2.
@emlun emlun self-requested a review December 11, 2024 09:41
Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well spotted, thanks! Unfortunately this still isn't valid CDDL, though. As I recently had clarified on the CDDL mail list, control operators only apply to types, not groups. This PR fixes the right-hand side, but the left-hand side is still a group, so the grammar is still invalid. I will open a meta-PR to complete the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants