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 RegEx translation of IP6 BNF, where [] denotes an option #837

Merged
merged 1 commit into from
Mar 1, 2016
Merged

Fix RegEx translation of IP6 BNF, where [] denotes an option #837

merged 1 commit into from
Mar 1, 2016

Conversation

ThomasR
Copy link
Contributor

@ThomasR ThomasR commented Mar 1, 2016

[ … ] in BNF must be (?: … )? in JS RegExp.

@Marsup Marsup added the bug Bug or defect label Mar 1, 2016
@Marsup Marsup self-assigned this Mar 1, 2016
@Marsup Marsup added this to the 8.0.4 milestone Mar 1, 2016
@Marsup
Copy link
Collaborator

Marsup commented Mar 1, 2016

Thanks a lot ! Lucky for us IPv6 is not that common yet.

Marsup added a commit that referenced this pull request Mar 1, 2016
Fix RegEx translation of IP6 BNF, where [] denotes an option
@Marsup Marsup merged commit 1bb38d4 into hapijs:master Mar 1, 2016
@ThomasR ThomasR mentioned this pull request Mar 1, 2016
@DavidTPate
Copy link
Contributor

I don't think this change was correct based on what I'm seeing in the specification. If you take a look at the host part you'll notice the mention of IP Literals you'll notice the following text:

A host identified by an Internet Protocol literal address, version 6
[RFC3513] or later, is distinguished by enclosing the IP literal
within square brackets ("[" and "]").

I see no mention around there of the brackets being optional. Could you point me to some documentation on this?

@DavidTPate
Copy link
Contributor

Let me clarify, I think this change was correct for the IPv6 validation, but it in turn messed up validation for URIs. So now URIs with IP literals that don't have the brackets would also be valid. I think some refactoring of this change just needs to occur so we have the correct setup for IP literals within a URI and also a single IP address.

@DavidTPate
Copy link
Contributor

Okay. Did some more looking at this, I'm being an idiot and misunderstood the change that was made here. At the surface I thought this was removing the brackets around the IP Literals, but I was entirely incorrect.

This is merely dealing with correcting the translation of the ABNF format for IPv6 here:

IPv6address = 6( h16 ":" ) ls32
/ "::" 5( h16 ":" ) ls32
/ [ h16 ] "::" 4( h16 ":" ) ls32
/[ *1( h16 ":" ) h16 ] "::" 3( h16 ":" ) ls32
/ [ *2( h16 ":" ) h16 ] "::" 2( h16 ":" ) ls32
/ [ *3( h16 ":" ) h16 ] "::" h16 ":" ls32
/ [ *4( h16 ":" ) h16 ] "::" ls32
/ [ *5( h16 ":" ) h16 ] "::" h16
/ [ *6( h16 ":" ) h16 ] "::"

Specifically the lines 3-9 which have [ h16 ] etc. meaning it is optional. Please disregard.

@ThomasR
Copy link
Contributor Author

ThomasR commented Mar 1, 2016

Just to clarify for anyone who wonders: [ , ] is for options while literal square brackets are written "[" , "]" in ABNF.

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants