1082723 - Make url.hostname not omit IPv6 delimiters

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Anne (:annevk)]

I thought this would be fixed by bug 960014 but it was not. E.g. if you have

  http://[2001:240:629::6]/foo.html

as URL printing its hostname will yield 2001:240:629::6 in Gecko whereas it's [2001:240:629::6] in Chromium (and the specification).

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Oh, this seems to be because window.location.hostname doesn't go through url.hostname :)
Looking into it. Also, it seems I might have to change Link::GetHostname too.

Thanks

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Attachment: Add IPv6 delimiters to nsLocation::GetHostname and Link::GetHostname

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Attachment: Remove expected FAIL for a-element IPv6 web-platform tests

Comment 4

[Anne (:annevk)]

Thanks, it seems strange our internal representation does not have the "[" and "]" and instead we rely on checking for a ":", but I guess it's okay.

Are those tests sufficient?

Comment 5

[Valentin Gosu [:valentin] (he/him)]

(In reply to Anne (:annevk) from comment #4)
> Thanks, it seems strange our internal representation does not have the "["
> and "]" and instead we rely on checking for a ":", but I guess it's okay.

Adding the [] to the internal representation is tricky, as consumers (such as the resolver) expect a non-brackety hostname. Changing how we use the hostname in Gecko is possible, however it adds in a bunch of duplicated code, and might break addons.

> 
> Are those tests sufficient?

http://[2001:240:629::6]/foo.html checks for the location.hostname, and the web-platform tests I enabled check for the Link::Hostname.

Comment 6

[Anne (:annevk)]

Ok, sounds good. I guess it doesn't matter if our internal representation doesn't match what we expose. It would be nice if they were more consistent, but not breaking compatibility seems way more important.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8505164 [details] [diff] [review]
Add IPv6 delimiters to nsLocation::GetHostname and Link::GetHostname

So we added similar code in
https://bugzilla.mozilla.org/attachment.cgi?id=8426316&action=edit

Could we please add a helper method for this.
Put it to nsContentUtils perhaps.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Attachment: Add IPv6 delimiters to nsLocation::GetHostname and Link::GetHostname

I moved the duplicated code to nsContentUtils. Thanks!

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8505459 [details] [diff] [review]
Add IPv6 delimiters to nsLocation::GetHostname and Link::GetHostname

Would be a bit simpler if the method took nsIURI* as in-param, and
nsAString& as out param.

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Attachment: Add IPv6 delimiters to nsLocation::GetHostname and Link::GetHostname

The function now takes a URI and puts out an nsAString - so it now does the UTF8ToUTF16 conversion as well.

Comment 11

[Valentin Gosu [:valentin] (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d911f85b517a
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea24186ca6c9

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d911f85b517a
https://hg.mozilla.org/mozilla-central/rev/ea24186ca6c9