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

Fix document.domain for IPv6 addresses to match browser behavior with respect to brackets #670

Closed
valenting opened this issue Feb 10, 2016 · 3 comments · Fixed by #678
Closed
Assignees

Comments

@valenting
Copy link

https://bugzilla.mozilla.org/show_bug.cgi?id=583170#c7

The spec [1] is vague when it defines the behaviour of document.domain for IPv6 domains, and when square brackets should be present.

[1] https://html.spec.whatwg.org/multipage/browsers.html#relaxing-the-same-origin-restriction

@domenic
Copy link
Member

domenic commented Feb 10, 2016

Could you be a bit more specific? It includes plenty of phrases like "If the new value is an IPv4 or IPv6 address", with the definition "If the document's domain starts with a U+005B LEFT SQUARE BRACKET character ([) and ends with a U+005D RIGHT SQUARE BRACKET character (]), it is an IPv6 address".

What is some example code setting or getting document.domain that you think the spec is unclear on how to process?

@bzbarsky
Copy link
Contributor

I don't think the spec is at all unclear here. It's very clear: document.domain on, say, http://[de67::314:5fff:fe52:83e0]:4555/test.html should return "de67::314:5fff:fe52:83e0".

The problem is that this does not match any browsers except Firefox (at least as reported in https://bugzilla.mozilla.org/show_bug.cgi?id=583170) and Firefox just changed its behavior to match the other browsers. So now the spec doesn't match any browsers at all for the getter.

There are also open questions about how the setter should behave; testing is needed to see what different browsers do here.

@domenic
Copy link
Member

domenic commented Feb 10, 2016

Let's repurpose the bug then! Renaming...

@domenic domenic changed the title Clarify behaviour of document.domain when navigating to an IPv6 address Fix document.domain for IPv6 addresses to match browser behavior with respect to brackets Feb 10, 2016
@annevk annevk self-assigned this Feb 10, 2016
annevk added a commit that referenced this issue Feb 11, 2016
This fixes #670 by no longer special casing IPv6. And rather than
mentioning IPv4 and IPv6 directly, we instead rely on the domain
concept.

It changes the definition of document.domain to rely on an internal
slot rather than somehow changing itself.

It makes use of the host parser rather than “domain to ASCII” which is
way too low-level to be used here.

Potential follow up here is when “registrable domain” gets defined as
per whatwg/url#72 so we no longer have to rely
on Public Suffix directly.
annevk added a commit that referenced this issue Feb 12, 2016
This fixes #670 by no longer special casing IPv6. And rather than
mentioning IPv4 and IPv6 directly, we instead rely on the domain
concept.

It changes the definition of document.domain to rely on an internal
slot rather than somehow changing itself.

It makes use of the host parser rather than “domain to ASCII” which is
way too low-level to be used here.

Potential follow up here is when “registrable domain” gets defined as
per whatwg/url#72 so we no longer have to rely
on Public Suffix directly.
annevk added a commit that referenced this issue Feb 12, 2016
This fixes #670 by no longer special casing IPv6. And rather than
mentioning IPv4 and IPv6 directly, we instead rely on the domain
concept.

It changes the definition of document.domain to rely on an internal
slot rather than somehow changing itself.

It makes use of the host parser rather than “domain to ASCII” which is
way too low-level to be used here.

Potential follow up here is when “registrable domain” gets defined as
per whatwg/url#72 so we no longer have to rely
on Public Suffix directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants