1195459 - nsStandardURL::SetHost should treat ipv6 hosts the same as nsStandardURL::GetHost

Status: NEW

(Core :: Security: CAPS, defect)

Description

[Nika Layzell [:nika] (ni? for response)]

Right now, due to bug 960014, the following occurs with hosts. This behavior is unintuitive and not in line with any other types of hosts.

var x = makeURI('http://[123:45::678]');
x.host                   // '123:45::678' - OK, [] isn't exactly part of the host.
x.hostPort               // '[123:45::678]' - good, you need the []s to disambiguate

x.host = '456:12::123';  // NS_ERROR_MALFORMED_URI - BAD, this could be the
                         // result of uri.host on a different ipv6 URI.
x.host = x.host;         // NS_ERROR_MALFORMED_URI - BAD, this works for all
                         // other kinds of hosts other than ipv6, and should 
                         // work for ipv6.
x.host = '[456:12::123]'
x.host                   // Will return '456:12::123', which is different than what it was set to. 

Either the host getter should be changed to always include the square brackets in the host, or the host setter should be changed to accept ipv6 URIs without the square brackets (and potentially reject ipv6 URIs with the square brackets, but I would be OK with accepting both). hostPort should maintain it's current behavior.

Both GetHost and SetHost should follow the same logic, and use the same number of []s around their values.

Comment 1

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

I think we tried changing it to work the correct way, but that would have meant auditing all of the call sites of SetHost/GetHost - some of that code does not expect the returned IP address to be enclosed in brackets.
It seems getters usually don't care about brackets, while setters almost always are aware of them. So we ended up with a compromise.

This is definitely something nice to have, but I wasn't able to find the best way to implement it.
Patches are always welcome :)

Comment 2

[Anne (:annevk)]

It seems this only affects the internal API as new URL(...).host does the correct thing as far as I can tell. In which case, feel free to change this however you want as long as you don't change what is exposed to the world.