Closed Bug 1082723 Opened 5 years ago Closed 5 years ago

Make url.hostname not omit IPv6 delimiters

Categories

(Core :: Networking, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: annevk, Assigned: valentin)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

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).
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
Assignee: nobody → valentin.gosu
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?
Blocks: url
(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.
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.
Attachment #8505165 - Flags: review?(bugs) → review+
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.
Attachment #8505164 - Flags: review?(bugs) → review-
I moved the duplicated code to nsContentUtils. Thanks!
Attachment #8505459 - Flags: review?(bugs)
Attachment #8505164 - Attachment is obsolete: true
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.
Attachment #8505459 - Flags: review?(bugs) → review-
The function now takes a URI and puts out an nsAString - so it now does the UTF8ToUTF16 conversion as well.
Attachment #8505846 - Flags: review?(bugs)
Attachment #8505459 - Attachment is obsolete: true
Attachment #8505846 - Flags: review?(bugs) → review+
You need to log in before you can comment on or make changes to this bug.