-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Refactor the document.domain attribute setter as a standalone algorithm #2365
Conversation
This is in response to W3C/HTML PR w3c/html#769. The Web Authentication WG's draft currently makes reference to the "Relaxing the same-origin restriction" of the document.domain attribute setter as a way to let relying parties use foo.bar.com to generate scoped credentials for bar.com. However, 1) the attribute setter procedure isn't documented as an algorithm - so we shouldn't call it like one, and 2) we need to override some of the ambient state within it, by changing some of the values to be passed as arguments. We had started some work to inline the procedure as an algorithm within our document, but consensus is that it'd be better if we could avoid future divergence by refactoring this part of the HTML spec instead.
Remove duplicated lines from the document.domain setter that are now handled in the standalone algorithm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good overall, but I have some nits and naming quibbles.
source
Outdated
|
||
<p>The <code data-x="dom-document-domain">domain</code> attribute's setter must run these | ||
<p>To <dfn data-export="">validate the relaxation of an origin</dfn> for an <span>origin</span> | ||
<var>currentOrigin</var> and a string <var>inputDomain</var>, the user agent must run these |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you basically trying to find out if origin's effective domain and input are a public suffix match? @mikewest and I have been talking about abstracting that operation into URL at some point. Maybe now is the time, although we can do it in two steps I suppose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I haven't found time to revive whatwg/url#72 yet, but I agree that it's a reasonable thing to do. I'll try to get to it next week when I'm back in the office.
source
Outdated
<code>DOMException</code>.</p></li> | ||
|
||
<li><p>If the given value is the empty string, then throw a | ||
<li><p>If <var>inputDomain</var> is the empty string, then throw a | ||
<span>"<code>SecurityError</code>"</span> <code>DOMException</code>.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we make this an abstract operation I don't think we should throw exceptions. Instead we should return failure and let the caller algorithm deal with the failure. That makes the algorithm more easily reusable in other contexts, that might not want to throw this exception.
source
Outdated
@@ -79482,50 +79482,22 @@ callback <dfn>FrameRequestCallback</dfn> = void (<span>DOMHighResTimeStamp</span | |||
|
|||
</dl> | |||
|
|||
<div w-nodev> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the new algorithm, provided we keep it in HTML, should be inside this <div w-nodev>
.
source
Outdated
<li><p>Let <var>host</var> be the result of <span data-x="validate the relaxation of an origin"> | ||
validating the relaxation of an origin</span> with this <code>Document</code> object's | ||
<span>origin</span> as <var>currentOrigin</var> and the given value as <var>inputDomain</var>. | ||
</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wrapping here is slightly wrong. The closing </p></li>
needs to be adjacent to the .
(which probably requires the last word on the previous line to wrap). Otherwise you get whitespace inbetween.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if we refactor as I suggested above, we need to account for failure here and throw an exception. If we don't, we need to state exceptions get rethrown.
- Moved the <div w-nodev> up. - Fixed a line break / mis-wrap in the attribute setter. - Moved all exception throwing outside the algorith, so it returns failure instead.
Thanks for the reviews! After consideration, would anyone rather I replace the PR's algorithm name |
I think the origin thing is a distraction and we should probably alter that argument. The real check that is being done is whether hostA is a Public Suffix match for hostB. I think calling it "Public Suffix match" or something that matches terminology that browsers use for this today would be clearer than talking about a relaxation. |
"hostA is a non-public suffix of hostB" or "hostA is a registrable domain suffix of hostB", ? These also have the benefit of being the same phrasing that a call would use, eliminating the need for The uses both take strings for hostA, so the algorithm should probably include the step of parsing the host. |
Renamed the algorithm "validate the relaxation of an origin" to "<var>hostA</var> is a registrable domain suffix of <var>hostB</var>", along with renaming the variables to hostA and hostB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Contributing my own minor style suggestions :). Overall this refactoring turned out much nicer than I anticipated from when I saw the previous threads; great stuff!
source
Outdated
@@ -79484,6 +79484,51 @@ callback <dfn>FrameRequestCallback</dfn> = void (<span>DOMHighResTimeStamp</span | |||
|
|||
<div w-nodev> | |||
|
|||
<p>To determine if <dfn data-export=""><var>hostA</var> is a registrable domain suffix of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is best as:
<p>To determine if a string <var>hostA</var> <dfn data-export="" data-lt="is a registrable domain
suffix|is not a registrable domain suffix">is a registrable domain suffix</dfn> of an
<span>origin</span> <var>hostB</var>, run these steps:</p>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use (with low priority since it's a bikeshed):
<p>A string <var>hostA</var> is <dfn data-export="">a
registrable domain suffix of</dfn> an <span>origin</span>
<var>hostB</var> if the following algorithm returns true:</p>
imitating https://html.spec.whatwg.org/multipage/browsers.html#same-origin. I omitted the "is" from the <dfn>
to avoid needing an lt
for "is not"
@annevk wanted hostB to be a host rather than an origin, which I think is ok, although it makes both call sites explicitly call https://html.spec.whatwg.org/multipage/browsers.html#concept-origin-effective-domain.
source
Outdated
@@ -79484,6 +79484,51 @@ callback <dfn>FrameRequestCallback</dfn> = void (<span>DOMHighResTimeStamp</span | |||
|
|||
<div w-nodev> | |||
|
|||
<p>To determine if <dfn data-export=""><var>hostA</var> is a registrable domain suffix of | |||
<var>hostB</var></dfn> for a string <var>hostA</var> and an <span>origin</span> <var>hostB</var>, | |||
the user agent must run these steps:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An algorithm named "is" should return true or false, not failure or a parsed host.
source
Outdated
|
||
<ol> | ||
<li><p>If <var>hostA</var> is the empty string, then throw a | ||
<span>"<code>SecurityError</code>"</span> <code>DOMException</code>.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should return failure (or false, per above)
source
Outdated
@@ -79484,6 +79484,51 @@ callback <dfn>FrameRequestCallback</dfn> = void (<span>DOMHighResTimeStamp</span | |||
|
|||
<div w-nodev> | |||
|
|||
<p>To determine if <dfn data-export=""><var>hostA</var> is a registrable domain suffix of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the arguments should be named hostString and origin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll give that a shot here in 2e67f75; it reads a little odd to say
To determine if a string hostString is a registrable domain suffix of an origin origin, run these steps:
but it looks like that style is used elsewhere in the document!
source
Outdated
value.</p></li> | ||
<li><p>Let <var>host</var> be the result of determining if <span><var>hostA</var> is a | ||
registrable domain suffix of <var>hostB</var></span> with the given value as <var>hostA</var> and | ||
this <code>Document</code> object's <span>origin</span> as <var>hostB</var>.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the above changes to true/false, then this and the next few steps would become:
<li><p>If the given value <span data-x="is a registrable domain suffix">is not a registrable
domain suffix</span> of this <code>Document</code> object's <span>origin</span>, then throw a
<span>"<code>SecurityError</code>"</span> <code>DOMException</code>.</p></li>
<li><p>Set this <code>Document</code> object's <span>origin</span>'s <span
data-x="concept-origin-domain">domain</span> to the result of <span data-x="host
parser">parsing</span> the given value.</p></li>
- Rename the algorithm to "is a registrable domain suffix" - Make the algorithm return true/false - Rename the arguments to hostString and origin
Thanks, all of you. This is far cleaner than it was yesterday! |
It seems @jyasskin did call out my feedback again but it got hidden due to the way GitHub deals with feedback. In particular I think the second argument should be a host too. I think that's what we want for cookies and what the underlying algorithm looks like in browsers, but it would be good if @mikewest could confirm before we made that change. (Anyone know of any other callers that are web-exposed (thus not browser UI)?) (And then I was also thinking of moving the algorithm to URL since that defines hosts, but maybe since HTML already defines the cookie API and |
I agree with this. I think we might want to define some URL-level concepts for more generally determining whether one host is a subdomain of another host (as per @devd's suggestions around https://twitter.com/frgx/status/831518747571675138), but we don't need to do that here.
https://tools.ietf.org/html/rfc6265#section-5.1.3 deals with hosts, so I'm fine with this algorithm dealing with hosts too. It's a little unfortunate that the callsite needs to know whether it wants to take the origin's domain into account, but I guess that's actually necessary since it's tough to make a generic decision about whether that applies to a specific instance. So, LGTM in general. |
Per annevk's review suggestion that this algorithm operate only on hosts, move the effectiveDomain function call to the outside of the algorithm.
Thanks again, all. I think 0f23e3a gets it right as to changing the second parameter into a host - I renamed it as |
I'd like to know if we want to stick with "registrable domain" even though both can also be IP addresses. That seems a little wrong and potentially confusing. I also want "effective domain" to return null for opaque origins and handle that case explicitly in the setter (currently you end up passing it along even though the suffix matching algorithm requires a string). I'm happy to make these final changes though to save you the trouble as we already went through a couple cycles here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion on whether it's ok to say that 1.2.3.4 is a registrable domain suffix of 1.2.3.4. The only other term we have from https://publicsuffix.org/list/ is "public suffix", which we'd have to negate into "1.2.3.4 is a non-public suffix of 1.2.3.4".
source
Outdated
@@ -79484,6 +79484,47 @@ callback <dfn>FrameRequestCallback</dfn> = void (<span>DOMHighResTimeStamp</span | |||
|
|||
<div w-nodev> | |||
|
|||
<p>To determine if a string <var>hostString</var> <dfn data-export="" data-lt="is a registrable domain | |||
suffix|is not a registrable domain suffix">is a registrable domain suffix</dfn> of a | |||
string <var>effectiveDomain</var>, run these steps:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should "string" be "host" here? https://url.spec.whatwg.org/#host-parsing seems to return non-strings for IP addresses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably. I wasn't sure that "host" was a valid type so I went with the one that I was sure was. Adjusting...
Suggestion: To determine if a host hostA is a registrable domain suffix of or is equal to hostB, ... |
So |
Yeah, plus " of" after "suffix" which I forgot before I edited my comment. The variables could simply be A and B to mirror https://url.spec.whatwg.org/#concept-host-equals. |
Oh, I guess we can't quite mirror it, since we do parse the input here. So you'd need to account for that somehow in the naming. Ugh, sorry. |
It seems like I should be able to leave out the
Also, the line for the |
Thanks, I'll do the remaining cleanup tomorrow and then leave it for a final review by everyone overnight and land it Wednesday. If anyone has any other feedback now would be a good time to post it. |
I made the remaining changes and also documented the known callers of this algorithm in case we need to refactor it later for other users. I think it's good to go now. |
source
Outdated
domain suffix of or is equal to|is not a registrable domain suffix of and is not equal to">is a | ||
registrable domain suffix of or is equal to</dfn> a <span data-x="concept-host">host</span> | ||
<var>targetHost</var>, run these steps:</p> | ||
<!-- Web platform usage: document.domain, Cookies, Web Authentication, Cookies --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to mention Cookies twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, will fix before landing tomorrow.
source
Outdated
<p>To determine if a string <var>hostString</var> <dfn data-export="" data-lt="is a registrable | ||
domain suffix of or is equal to|is not a registrable domain suffix of and is not equal to">is a | ||
registrable domain suffix of or is equal to</dfn> a <span data-x="concept-host">host</span> | ||
<var>targetHost</var>, run these steps:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@annevk, "targetHost" seems confusing here. When we use this in document.domain and webauthn, the "target" for the host value, that is, the host the caller wants to achieve, is passed in "hostString", while the current or original host is in "targetHost".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't really sure what name to use. Do you have a better alternative? Note that if you suggest just "host", you need to change the steps of the algorithm somehow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(An alternative would be to let the caller do the parsing and then you'd just do A and B as inputs.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use "originalHost" or "fullHost" for the larger host, and "hostSuffix" (and "hostSuffixString") for the potential suffix. "A" and "B" are ok enough that I didn't complain before, but they still look more symmetric than I want for a fundamentally asymmetric algorithm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hostSuffixString and originalHost seem reasonable to me.
source
Outdated
@@ -79484,6 +79484,49 @@ callback <dfn>FrameRequestCallback</dfn> = void (<span>DOMHighResTimeStamp</span | |||
|
|||
<div w-nodev> | |||
|
|||
<p>To determine if a string <var>hostString</var> <dfn data-export="" data-lt="is a registrable | |||
domain suffix of or is equal to|is not a registrable domain suffix of and is not equal to">is a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will commas work in link-text? If so, doing this would help readability..
data-lt="is a registrable domain suffix of, or is equal to,|
is not a registrable domain suffix of, and is not equal to,"
Regarding the "registrable domain suffix" term: it is essentially inventing yet another term for "public suffix plus one (additional label)" (c.f., "Algorithm" step 7 of [1] https://publicsuffix.org/list/), a.k.a. "effective top-level domain plus one (eTLD+1)". The terms "registered domain" and "registrable domain" are not clearly defined in [1] and I've had a discussion with [email protected] resulting in this issue and related PR. After thinking about it some, I suppose "registrable domain suffix" is OK given we lack something clearly better, and have commented on the PR regarding altering the proposed definition of "registrable domain" such that it works in this context -- I am assuming that this algorithm does not know, or need to know, whether the originalHost
is presently DNS-resolvable or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I took "The registered or registrable domain is the public suffix plus one additional label.", step 7 of the public suffix list algorithm, as the definition of "registrable domain" and "registered domain", in the same way that step 6 is the definition of the "public suffix".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a trailing comma in the term seems weird...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jyasskin yeah but it (Algorithm" step 7 of [1] https://publicsuffix.org/list/) is an insufficient and unclear definition, as it presently stands, because it does not explain the connotations of "registered" or "registrable".
thanks to y'all for helping with this! |
To answer the overnight questions:
I will land this later today, unless there's an another error or objection. |
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Updates: Refactored two new protected methods to avoid code duplication. [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez --HG-- extra : rebase_source : 5d042c4e97b8866027c81ea0f1c544ce1721b7c4
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Update 1: Refactored two new protected methods to avoid code duplication. Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the provided domain so as to catch internationalized domains. Update 3: Nit-fix and rebase [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez --HG-- extra : rebase_source : 634bd3c7b60c7ad996ee38ab7314071b426e3f6f
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Update 1: Refactored two new protected methods to avoid code duplication. Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the provided domain so as to catch internationalized domains. Update 3: Nit-fix and rebase [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Updates: Refactored two new protected methods to avoid code duplication. [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Updates: Refactored two new protected methods to avoid code duplication. [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez UltraBlame original commit: ffa2f50d49ce0356bbf02ea0940060283dcc007c
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Update 1: Refactored two new protected methods to avoid code duplication. Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the provided domain so as to catch internationalized domains. Update 3: Nit-fix and rebase [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez UltraBlame original commit: 1147dd18f1d91034757ea50a01281fae6e43eda6
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Updates: Refactored two new protected methods to avoid code duplication. [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez UltraBlame original commit: ffa2f50d49ce0356bbf02ea0940060283dcc007c
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Update 1: Refactored two new protected methods to avoid code duplication. Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the provided domain so as to catch internationalized domains. Update 3: Nit-fix and rebase [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez UltraBlame original commit: 1147dd18f1d91034757ea50a01281fae6e43eda6
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Updates: Refactored two new protected methods to avoid code duplication. [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez UltraBlame original commit: ffa2f50d49ce0356bbf02ea0940060283dcc007c
…lTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Update 1: Refactored two new protected methods to avoid code duplication. Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the provided domain so as to catch internationalized domains. Update 3: Nit-fix and rebase [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] whatwg/html#2365 [3] w3ctag/design-reviews#97 (comment) MozReview-Commit-ID: 4Dr8yOMdhez UltraBlame original commit: 1147dd18f1d91034757ea50a01281fae6e43eda6
This is in response to W3C/HTML PR w3c/html#769.
The Web Authentication WG's draft currently makes reference to the "Relaxing the same-origin restriction" of the
document.domain
attribute setter as a way to let relying parties (app developers) usefoo.bar.com
to generate scoped credentials forbar.com
.However:
We had started some work to inline the procedure as an algorithm within our document, but consensus is that it'd be better if we could avoid future divergence by refactoring this part of the HTML spec instead.
Note to reviewers: This PR is split into 3 parts for -- I hope -- easier review. The first commit copies the salient parts of the
document.domain
attribute setter into a new algorithm to "validate an origin relaxation". The second commit removes the duplicated parts from the attribute setter. The final commit adds me to the acknowledgements. I recommend reviewing them individually.