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

[connect-tcp] Define syntax more precisely #2719

Closed
wants to merge 4 commits into from
Closed

Conversation

bemasc
Copy link
Contributor

@bemasc bemasc commented Jan 25, 2024

The language here is adapted from RFC 9298.

Fixes #2714

The language here is adapted from RFC 9298.

Fixes #2714
ip-addr = IPv6address / IPv4address
single-host-str = DQUOTE ( ip-addr / reg-name ) DQUOTE
ip-addr-str = DQUOTE ip-addr DQUOTE
ip-addr-list = "(" ( ip-addr-str ", " )* ip-addr-str ")"
Copy link
Contributor

Choose a reason for hiding this comment

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

This ABNF doesn't match the example, it includes DQUOTES and parentheses that aren't in the example. Why not simply reuse the syntax from 9298 with just the tweak for the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the quotes and parentheses are from RFC 6570's value syntax. They're needed in order to distinguish lists from strings, since ABNF has no concept of a list.

It sounds like you'd prefer to use ABNF only for the individual list elements, and leave the description of the list itself in English?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess I'm confused, are the double quotes and parentheses required? If so, can you add them to the examples so I can visualize the syntax better?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm even more confused by 5f5e1ac. I assumed that the ABNF covered what would end up in the path, not in the "Variable assignments in RFC 6570 syntax". What am I missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that it's confusing. ABNF is not really able to do what we need here. I've moved more of the description to English to reduce the complexity.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not? Doesn't this work?

ip-addr = IPv6address / IPv4address
ip-addr-list = *( ip-addr "," ) ip-addr
target_host = reg-name / ip-addr-list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an option, but it would result in a change in the behavior because it would not respect the explode modifier. I've added more examples that I hope make this clear.

RFC 6570 does not really define a canonical string representation for lists, so we can't specify them using ABNF.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. If it were up to me, I would remove the explode modifier and only use the ABNF - that way we only have a single way to convey the information, and it's formally defined.

@tfpauly tfpauly added the connect-tcp draft-ietf-httpbis-connect-tcp label Feb 14, 2024
@bemasc
Copy link
Contributor Author

bemasc commented Feb 14, 2024

Closing in favor of #2736

@bemasc bemasc closed this Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connect-tcp draft-ietf-httpbis-connect-tcp
Development

Successfully merging this pull request may close these issues.

connect-tcp: target_host is under-defined
3 participants