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

Clarifications about TLS Names and ABNF of ports #122

Merged

Conversation

swcurran
Copy link
Collaborator

Addresses PRs #110 and #111.

Please review carefully the ABNF -- I think I have it right, but could use a second set of eyes on it.

@daidoji and @andrewwhitehead (and anyone else expert in ABNF).

Signed-off-by: Stephen Curran swcurran@gmail.com

Signed-off-by: Stephen Curran <swcurran@gmail.com>
@swcurran swcurran requested review from a team October 24, 2024 23:48
PatStLouis
PatStLouis previously approved these changes Oct 25, 2024
domain-segment = ; A part of a domain name as defined in RFC3986, such as "example" and "com" in "example.com"
percent-encoded-port = "%3A" ( "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" ) 1*3( "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" )
Copy link

Choose a reason for hiding this comment

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

I think it should be 1*4 here as ports can go up to five digits. Also there's a commonly used ABNF primitive called "DIGIT" you can use instead of listing all the values.

So its something like ["%3A" 1*5(DIGIT)] should be fine if you'd rather do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was being picky :-). The port (AFAIK) can’t have a leading 0 and “00000” can’t be used, hence the extra.

I didn’t know about 5 digit ports, but I guess that is obvious — 0 - 64k. So there you go.

Updating to: percent-encoded-port = "%3A" ( "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" ) 1*4( DIGIT )

What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated. Realized at the same time that the scid component was missing.

Signed-off-by: Stephen Curran <swcurran@gmail.com>
Signed-off-by: Stephen Curran <swcurran@gmail.com>
@swcurran swcurran merged commit c54fcc3 into decentralized-identity:main Oct 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants