url: % is a valid host character in IPv6 addresses #9411

cthayer · 2015-03-14T07:10:28Z

% is a valid host character in IPv6 link-local addresses.

The % separates the address from the zone within the hostname section of the URL.

This fixes issue #9404

non-host chars are different for IPv6 addresses vs other hostnames detect the presence of an IPv6 address hostname before doing the non-host char check and select the non-host char set accordingly

% is a valid character in IPv6 link-local addresses. Test to make sure that these types of addresses are parsed correctly.

misterdjules · 2015-03-16T17:11:57Z

Adding to milestone 0.12.2 for the same reason its corresponding issue has been added to 0.12.2 too.

jbergstroem · 2015-03-23T23:14:56Z

Isn't % supposed to be represented using percent-encoding (%25) in uri's? http://tools.ietf.org/html/rfc6874#section-2

trevnorris · 2015-03-23T23:23:12Z

Do any browsers support this?

EDIT: I mean do any browsers properly support parsing this. e.g.

var a = a.document.createElement('a');
a.href = 'http://[fe80::1%25lo0]:51877';
console.log(a.host);  // output: ":0"

bahamat · 2015-03-23T23:36:18Z

Curl, at least, does.

$ curl -i 'http://[fe80::253:a3ff:fe13:7081%en4]:80/'
HTTP/1.1 200 OK
Date: Mon, 23 Mar 2015 23:34:29 GMT
Server: Apache/2.4.9 (Unix)
Content-Location: index.html.en
Vary: negotiate
TCN: choice
Last-Modified: Mon, 11 Jun 2007 18:53:14 GMT
ETag: "2d-432a5e4a73a80"
Accept-Ranges: bytes
Content-Length: 45
Content-Type: text/html

<html><body><h1>It works!</h1></body></html>

cthayer · 2015-03-24T00:12:23Z

Node.js allows the % character as well (just use a net.Socket to try it out)

This is just an issue with url.parse correctly parsing a URL with an IPv6 Link Local address in it.

misterdjules · 2015-04-29T00:23:33Z

Corresponding issues in Chromium's issues tracker and in Firefox's issue tracker.

It seems that both of them are seriously considering implementing RFC 6874, but it hasn't been done yet.

Removing from the 0.12.4 milestone as currently it's not supported by most major browsers, and thus doesn't seem to be a pressing issue. It seems it would make sense to have Node.js conform to that RFC though.

cthayer · 2015-04-29T14:50:31Z

I must say that it's a little frustrating having feature support (or bug fixes) for a server side language dictated by browser support.

Currently, it's impossible to bind a server or connect a client to a link local address on the same machine when using any npm module that sanitizes it's URL's via url.parse because url.parse does not respect that everything within the [ and ] is the IPv6 address.

Take url.parse out of the equation and everything works. To me, that means that node.js already supports RFC 6874 it's just url.parse that doesn't.

This would sit much better with me if my tiny changes were breaking tests or something else rather than this change not being implemented (or deemed important) because browsers don't support this yet. Last I checked, node.js was a server side language and this issue essentially removes npm as a resource for a project I'm working on.

</rant>

Just wanted to get my thoughts/feelings out there. Just frustrated as this issue is a big issue for me.

trevnorris · 2015-04-29T23:35:37Z

If the 0.12.4 release is pressing then let's at least get the fix in for the 0.12.5 release.

misterdjules · 2015-04-30T00:21:25Z

@cthayer @trevnorris v0.12.4 is not pressing, as we're currently working on v0.12.3. I don't have anything against adding this PR back to the 0.12.4 milestone if the impact of its changes are well understood. My understanding was that it was not the case.

I also initially thought we had more issues still open in the 0.12.3 and 0.12.4 milestones and didn't want to add much more to it except for major issues that are well understood, but looking at them again it seems there's still some room for other issues.

I apologize for the confusion.

trevnorris · 2015-04-30T20:36:33Z

If this helps the parser more closely follow the spec, and it's only an additive change, then I don't see this merging this as an issue.

@misterdjules You cool if this is added to the 0.12.4 milestone?

misterdjules · 2015-04-30T21:49:32Z

@trevnorris Yes, I'm ok with adding it to the 0.12.4 milestone as long as we consider it's not an addition to the API, but a bug fix. In my opinion it is a bug fix.

@trevnorris @cthayer As usual we still need to do a thorough review and after briefly looking at the code changes again I would already recommend adding more tests for invalid input, like an url with a valid IPv6 address, a % character but without a zone id.

cthayer · 2015-05-01T13:54:38Z

@trevnorris @misterdjules Thanks for the additional input. I'm glad to see that this is being considered for the 0.12.4 release again. I'm happy to add more tests.

I hadn't done actual validation of the hostname portion when a IPv6 zone-id is involved because it didn't appear that url.parse does any validation of the hostname portion, rather it just finds the left boundary of the hostname portion and I was trying to make my change as minimal as possible. So I know that my code would fail a test for an IPv6 address with a % and no zone-id! :)

I'm all for making sure things are correct, so I'll add more tests and more logic for validating the zone-id. Also, since I'm validating the zone-id, is it cool if I add a parameter to the output called zoneId that contains just the IPv6 zone-id (if present)?

misterdjules · 2015-05-06T00:20:55Z

@cthayer Thank you for considering to add more tests! Just to make sure we're on the same page, I was not suggesting to validate IPv6 addresses if that's not already done for host names, just that a zone-id is provided after a % character if that's required by the RFC. In other words, having just one test seems like it's probably not enough.

I would suggest adding that additional zoneId property to the output in another PR targeted to master. Even if it wouldn't break backward compatibility, I'd rather keep changing the API on development versions, unless @trevnorris or @joyent/node-coreteam disagrees.

Thank you!

Add a test for IPv6 link local addresses that contain the '%' character, but are missing the zone id.

If the IPv6 hostname is a link local address that is missing the zone id (ends in '%]'), then remove the '%' character from the hostname

cthayer · 2015-05-19T14:03:11Z

@misterdjules I've added a test (and supporting code) for a url with a valid IPv6 address, a % character but without a zone id.

My solution was to strip the % character from the hostname and continue on. So, for example, [::1%] becomes [::1].

I wasn't sure how much hostname validation you want to add, since there wasn't much there before, so I left it at that, but let me know if you'd like to be more thorough.

Craig Thayer added 2 commits March 12, 2015 10:40

improve url.parse support for IPv6 addresses

7108142

non-host chars are different for IPv6 addresses vs other hostnames detect the presence of an IPv6 address hostname before doing the non-host char check and select the non-host char set accordingly

url: test for % in IPv6 link-local addresses

78e2d3c

% is a valid character in IPv6 link-local addresses. Test to make sure that these types of addresses are parsed correctly.

Nodejs-Jenkins added pr master labels Mar 14, 2015

cthayer mentioned this pull request Mar 14, 2015

url.parse does not handle IPv6 link-local addresses correctly #9404

Closed

misterdjules added this to the 0.12.2 milestone Mar 16, 2015

misterdjules modified the milestones: 0.12.4, 0.12.3 Apr 1, 2015

misterdjules removed this from the 0.12.4 milestone Apr 29, 2015

misterdjules added this to the 0.12.4 milestone May 6, 2015

Craig Thayer added 2 commits May 19, 2015 06:40

add test for IPv6 link local addresses

4a0dd2e

Add a test for IPv6 link local addresses that contain the '%' character, but are missing the zone id.

IPv6 link local address hostname validation

63972b1

If the IPv6 hostname is a link local address that is missing the zone id (ends in '%]'), then remove the '%' character from the hostname

misterdjules modified the milestones: 0.12.4, 0.12.5 May 25, 2015

misterdjules modified the milestones: 0.12.5, 0.12.6 Jun 22, 2015

misterdjules modified the milestones: 0.12.6, 0.12.7, 0.12.8 Jul 6, 2015

pdcastro mentioned this pull request Jul 15, 2019

balena scan fails to list devices where avahi advertises IPv6 addresses balena-io/balena-cli#1350

Open

Trott closed this Apr 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

url: % is a valid host character in IPv6 addresses #9411

url: % is a valid host character in IPv6 addresses #9411

cthayer commented Mar 14, 2015

misterdjules commented Mar 16, 2015

jbergstroem commented Mar 23, 2015

trevnorris commented Mar 23, 2015

bahamat commented Mar 23, 2015

cthayer commented Mar 24, 2015

misterdjules commented Apr 29, 2015

cthayer commented Apr 29, 2015

trevnorris commented Apr 29, 2015

misterdjules commented Apr 30, 2015

trevnorris commented Apr 30, 2015

misterdjules commented Apr 30, 2015

cthayer commented May 1, 2015

misterdjules commented May 6, 2015

cthayer commented May 19, 2015

url: % is a valid host character in IPv6 addresses #9411

url: % is a valid host character in IPv6 addresses #9411

Conversation

cthayer commented Mar 14, 2015

misterdjules commented Mar 16, 2015

jbergstroem commented Mar 23, 2015

trevnorris commented Mar 23, 2015

bahamat commented Mar 23, 2015

cthayer commented Mar 24, 2015

misterdjules commented Apr 29, 2015

cthayer commented Apr 29, 2015

trevnorris commented Apr 29, 2015

misterdjules commented Apr 30, 2015

trevnorris commented Apr 30, 2015

misterdjules commented Apr 30, 2015

cthayer commented May 1, 2015

misterdjules commented May 6, 2015

cthayer commented May 19, 2015