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

IPv6 host normalization. #1560

Closed
ioquatix opened this issue Feb 5, 2020 · 10 comments · Fixed by #1561
Closed

IPv6 host normalization. #1560

ioquatix opened this issue Feb 5, 2020 · 10 comments · Fixed by #1561
Milestone

Comments

@ioquatix
Copy link
Member

ioquatix commented Feb 5, 2020

I reviewed the PR #1213 and it looks like we recently merged #1538 which prefered the non-square-brackets representation. However, this has not been released yet. So we could adjust it.

Based on the defintion of URI#host, I think we should probably prefer the square brackets representation... but I'm not sure what is canonical. I need to do some research.

Originally posted by @ioquatix in #1213 (comment)

@ioquatix
Copy link
Member Author

ioquatix commented Feb 5, 2020

@jeremyevans said:

RFC 7239 Section 7.4:

   Note that IPv6 addresses may not be quoted in
   X-Forwarded-For and may not be enclosed by square brackets, but they
   are quoted and enclosed in square brackets in "Forwarded".

       X-Forwarded-For: 192.0.2.43, 2001:db8:cafe::17

   becomes:

       Forwarded: for=192.0.2.43, for="[2001:db8:cafe::17]"

So I think we should handle this for X-Forwarded-*. but not Forwarded (when support is added
for Forwarded #1423).

To me, Forwarded header is more recent, and it prefers quotes.

Additionally, a URI#host field is quoted:

URI.parse("http://[2001:db8:cafe::17]:47011/").host
=> "[2001:db8:cafe::17]"

That being said, you can't feed such a value to Addrinfo.tcp. Nor do command line tools prefer it:

koyoko% ping "[2406:e003:1d0f:fa00:ec4:7aff:feb5:9500]"
ping: [2406:e003:1d0f:fa00:ec4:7aff:feb5:9500]: Name or service not known
koyoko% ping "2406:e003:1d0f:fa00:ec4:7aff:feb5:9500" 
PING 2406:e003:1d0f:fa00:ec4:7aff:feb5:9500(2406:e003:1d0f:fa00:ec4:7aff:feb5:9500) 56 data bytes
64 bytes from 2406:e003:1d0f:fa00:ec4:7aff:feb5:9500: icmp_seq=1 ttl=61 time=4.45 ms

On https://bugs.ruby-lang.org/issues/14016 it states:

Use URI::HTTP.hostname which unwraps the bracket.

u = URI("http://[::1]/bar")
p u.hostname      #=> "::1"
p u.host          #=> "[::1]"

So, maybe we should adopt this model.

@ioquatix ioquatix added this to the v2.2.0 milestone Feb 5, 2020
@ioquatix
Copy link
Member Author

ioquatix commented Feb 5, 2020

@AlexWayfer do you have any opinion on this?

@AlexWayfer
Copy link
Contributor

AlexWayfer commented Feb 5, 2020

@AlexWayfer do you have any opinion on this?

To be honest, no, because as I wrote in #1538:

I'm not sure about IPv6 format, you can correct tests and I'll adopt the code.

I'm not familiar with IPv6 standards and formats.

@ioquatix
Copy link
Member Author

ioquatix commented Feb 5, 2020

I want to follow the same naming conventions as URI#hostname and URI#host, along with the canonical definitions of the authority header in HTTP/2. I am working on a PR but there are a couple of tricky edge cases!

@AlexWayfer
Copy link
Contributor

I want to follow the same naming conventions as URI#hostname and URI#host, along with the canonical definitions of the authority header in HTTP/2. I am working on a PR but there are a couple of tricky edge cases!

You always can publish draft PR and we can try to help you with them.

@ioquatix
Copy link
Member Author

ioquatix commented Feb 5, 2020

My goal is to use the following terminology:

  • Authority (as per HTTP/2 spec): provided by host header, should be formatted as address:port, ipv4address:port or [ipv6adderss]:port

  • Host (as per URI spec): the address part including delimiters, without port.

  • Hostname (as per URI spec): the address part excluding delimiters, without port.

@AlexWayfer
Copy link
Contributor

Hostname (as per URI spec): the address part excluding delimiters, without port.

Does "delimeters" mean square parentheses?

I wanted a helper method for host without port or with it (as it in headers) in #1435, and we've chose hostname for this.

@ioquatix
Copy link
Member Author

ioquatix commented Feb 5, 2020

@AlexWayfer Yeah delimiters means square parentheses.

Yeah, agreed we need some better accessors.

Request#hostname should be the thing you can pass to Addrinfo.tcp.

@ioquatix
Copy link
Member Author

ioquatix commented Feb 6, 2020

I've reworked the PR, so that the first commit is the failing specs.

@AlexWayfer
Copy link
Contributor

I've reworked the PR, so that the first commit is the failing specs.

I see now specs are passing. 🎉

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 a pull request may close this issue.

2 participants