Implement setter for document.domain #15536

asajeffrey · 2017-02-13T20:53:43Z

This PR implements the setter for document.domain.

It builds on #15438 and #15478, only the last commit is part of this PR.

It includes tests for similar-origin security checks.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #934.
There are tests for these changes.

This change is

highfive · 2017-02-13T20:53:48Z

Heads up! This PR modifies the following files:

@fitzgen: components/script/dom/servoparser/mod.rs, components/script/dom/window.rs, components/script/dom/location.rs, components/script/dom/document.rs, components/script/dom/xmlhttprequest.rs, components/script/devtools.rs, components/script/dom/history.rs, components/script/webdriver_handlers.rs, components/script/lib.rs, components/script/dom/domparser.rs, components/script/script_thread.rs, components/script/dom/domimplementation.rs, components/script/dom/htmliframeelement.rs, components/script/dom/webidls/Location.webidl, components/script/dom/node.rs, components/script/dom/xmldocument.rs, components/script/dom/webidls/Document.webidl, components/script/dom/bindings/trace.rs, components/script/origin.rs
@KiChjang: components/script/dom/servoparser/mod.rs, components/script/dom/window.rs, components/script/dom/location.rs, components/script/dom/document.rs, components/net/http_loader.rs, components/script/dom/xmlhttprequest.rs, components/script/devtools.rs, components/script/dom/history.rs, components/script/webdriver_handlers.rs, components/script/lib.rs, components/script/dom/domparser.rs, components/script/script_thread.rs, components/script/dom/domimplementation.rs, components/script/dom/htmliframeelement.rs, components/script/dom/webidls/Location.webidl, components/script/dom/node.rs, components/script/dom/xmldocument.rs, components/script/dom/webidls/Document.webidl, components/script/dom/bindings/trace.rs, components/script/origin.rs, components/net_traits/request.rs, components/net_traits/request.rs

highfive · 2017-02-13T20:53:49Z

Warning

These commits include an empty title element (<title></title>). Consider adding appropriate metadata.
These commits modify unsafe code. Please review it carefully!

asajeffrey · 2017-02-13T20:54:20Z

cc @jdm @Ms2ger

asajeffrey · 2017-02-15T22:16:25Z

Updated the wpt manifest.

bors-servo · 2017-02-18T21:28:21Z

☔ The latest upstream changes (presumably #15588) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2017-02-23T02:43:18Z

☔ The latest upstream changes (presumably #14962) made this pull request unmergeable. Please resolve the merge conflicts.

asajeffrey · 2017-02-27T02:08:10Z

IRC conversation with @jdm: http://logs.glob.uno/?c=mozilla%23servo&s=24+Feb+2017&e=24+Feb+2017#c619500

TL;DR: closed #15478 in favour of merging with this PR, so we can include wpt tests.

asajeffrey · 2017-02-27T03:17:43Z

The spec for document.domain changed: whatwg/html@03bbc6e

I added some more wpt tests.

@emilio, @Ms2ger and @jdm: This is ready for review, since #15478 was closed in favour of this PR.

asajeffrey · 2017-02-27T16:00:19Z

The spec for document.open() changed: whatwg/html#2288

IRC chat with @Ms2ger: http://logs.glob.uno/?c=mozilla%23servo&s=27+Feb+2017&e=27+Feb+2017#c620440 - now that there's a wpt test for document.open() we should have sufficient test coverage.

Ms2ger · 2017-03-02T14:49:19Z

components/script/dom/webidls/Document.webidl

@@ -81,7 +81,7 @@ partial /*sealed*/ interface Document {
  // resource metadata management
  [/*PutForwards=href, */Unforgeable]
  readonly attribute Location? location;
-  readonly attribute DOMString domain;
+  [Throws] attribute DOMString domain;


Can this use [SetterThrows]?

Oh, I didn't know there was such a thing! Yes, it should.

asajeffrey · 2017-03-02T19:58:57Z

Fixes #15233.

asajeffrey · 2017-03-08T16:33:07Z

Squashed down into two commits. The first used to be #15478, the second is the implementation of the setter for document.domain.

asajeffrey · 2017-03-14T19:36:48Z

Pushed again to get appveyor to rebuild.

nox · 2017-03-15T09:01:44Z

components/net_traits/pub_domains.rs

+            Some(Host::Domain(String::from(reg_suffix(&*domain))))
+        } else {
+            Some(host)
+        },


This is worse than before or what I suggested and the formatting is wrong. Don't nest an if let expression inside a match arm without enclosing it in braces.

nox · 2017-03-15T09:01:44Z

components/script/dom/document.rs

-fn is_a_registrable_domain_suffix_of_or_is_equal_to(host_suffix_string: &str, original_host: Host) -> bool {
+// The spec says to return a bool, we actually return an Option<Host> containing
+// the parsed host in the successful case, to avoid having to re-parse the host.
+fn is_a_registrable_domain_suffix_of_or_is_equal_to(host_suffix_string: &str, original_host: Host) -> Option<Host> {


Rename the method to something that doesn't sound like a boolean.

nox · 2017-03-15T09:01:44Z

components/script/dom/document.rs

+        let host = match is_a_registrable_domain_suffix_of_or_is_equal_to(&*value, effective_domain) {
+            None => return Err(Error::Security),
+            Some(host) => host,
+        };


let host = is_a_registrable_domain_suffix_of_or_is_equal_to(&*value, effective_domain).ok_or(Error::Security)?;

nox · 2017-03-15T09:02:16Z

@bors-servo try

bors-servo · 2017-03-15T09:02:22Z

⌛ Trying commit 7c91817 with merge ea09005...


        Auto merge of #15536 - asajeffrey:script-document-domain-setter, r=<try>

Implement setter for document.domain  This PR implements the setter for `document.domain`. It builds on #15438 and #15478, only the last commit is part of this PR. It includes tests for similar-origin security checks. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #934. - [X] There are tests for these changes.   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15536)

bors-servo · 2017-03-15T09:28:05Z

💔 Test failed - linux-rel-wpt

nox · 2017-03-15T14:08:16Z

components/net_traits/pub_domains.rs

+            _ => Some(host),
+        },
+        ImmutableOrigin::Opaque(_) => None,
+    }


Still the same issue?

Either just copy/paste my original comment, or wrap the nested match in braces.

match url.origin() { ImmutableOrigin::Tuple(_, Host::Domain(domain), _) => Some(Host::Domain(String::from(reg_suffix(&*domain)))), ImmutableOrigin::Tuple(_, ip, _) => Some(ip), ImmutableOrigin::Opaque(_) => None, }

Or a mix of the two.

match url.origin() { ImmutableOrigin::Tuple(_, Host::Domain(domain), _) => { Some(Host::Domain(String::from(reg_suffix(&*domain)))) }, ImmutableOrigin::Tuple(_, ip, _) => Some(ip), ImmutableOrigin::Opaque(_) => None, }

nox · 2017-03-15T14:28:43Z

r=me when the tests pass.

nox · 2017-03-15T14:44:59Z

The intermittent crash seems to be a race condition when writing the alert() message, please add a security check to it so that it will throw SecurityError as the spec mandates in the test, thus avoiding the intermittent crash entirely.

asajeffrey · 2017-03-15T15:17:52Z

Added #15962.


        Implement setter for document.domain

asajeffrey · 2017-03-15T15:35:38Z

Squashed down to two commits.

@bors-servo: r=nox

bors-servo · 2017-03-15T15:35:39Z

📌 Commit 5348b63 has been approved by nox

bors-servo · 2017-03-15T15:35:43Z

⌛ Testing commit 5348b63 with merge e600e04...


        Auto merge of #15536 - asajeffrey:script-document-domain-setter, r=nox

Implement setter for document.domain  This PR implements the setter for `document.domain`. It builds on #15438 and #15478, only the last commit is part of this PR. It includes tests for similar-origin security checks. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #934. - [X] There are tests for these changes.   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15536)

bors-servo · 2017-03-15T16:16:04Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev
Approved by: nox
Pushing e600e04 to master...


        Auto merge of #15799 - asajeffrey:script-window-xorigin-parent, r=nox

Implement dissimilar-origin window.parent and window.top  This PR implements `window.parent` and `window.top` for dissimilar-origin windows. This PR builds on #15536, only the last commit is part of this PR. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #14996 and fix #11660. - [X] These changes do not require tests because there's already a parentage test in `mozilla/cross-origin-objects/cross-origin-objects.html`.   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15799)


        Implement setter for document.domain

Upstreamed from servo/servo#15536 [ci skip]

asajeffrey added A-content/script A-security labels Feb 13, 2017

highfive assigned emilio Feb 13, 2017

highfive added the S-awaiting-review label Feb 13, 2017

asajeffrey changed the title ~~Script document domain setter~~ Implement setter for document.domain Feb 13, 2017

This was referenced Feb 13, 2017

Implement document.domain #934

Closed

Added some same-origin-domain checks #15478

Closed

asajeffrey force-pushed the asajeffrey:script-document-domain-setter branch from e33dace to e2d4a51 Feb 15, 2017

highfive added the S-needs-rebase label Feb 18, 2017

asajeffrey force-pushed the asajeffrey:script-document-domain-setter branch from e2d4a51 to d68a989 Feb 22, 2017

highfive added the S-needs-rebase label Feb 23, 2017

asajeffrey force-pushed the asajeffrey:script-document-domain-setter branch from d68a989 to 34f1164 Feb 23, 2017

asajeffrey removed the S-needs-rebase label Feb 27, 2017

asajeffrey force-pushed the asajeffrey:script-document-domain-setter branch from 94ae9b2 to 7ce7340 Feb 27, 2017

Ms2ger reviewed Mar 2, 2017

View changes

asajeffrey mentioned this pull request Mar 2, 2017

Implement dissimilar-origin window.parent and window.top #15799

Merged

4 of 4 tasks complete

asajeffrey mentioned this pull request Mar 2, 2017

Implement cross-origin iframe.contentWindow #15233

Closed

nox added the S-needs-squash label Mar 3, 2017

asajeffrey force-pushed the asajeffrey:script-document-domain-setter branch from 971794d to 4d82e40 Mar 8, 2017

nox requested changes Mar 15, 2017

View changes

highfive added S-tests-failed and removed S-tests-failed labels Mar 15, 2017

nox reviewed Mar 15, 2017

View changes

nox approved these changes Mar 15, 2017

View changes

Implement setter for document.domain

Loading status checks…

5348b63

asajeffrey force-pushed the asajeffrey:script-document-domain-setter branch from 0ae1849 to 5348b63 Mar 15, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Mar 15, 2017

highfive removed the S-awaiting-merge label Mar 15, 2017

This was referenced Mar 15, 2017

Implement cross-thread postMessage #15679

Merged

[WIP] Make subsequent about:blank loads async #14764

Closed

jdm added a commit to web-platform-tests/wpt that referenced this pull request Apr 17, 2017

Implement setter for document.domain

Loading status checks…

145ebf2

Upstreamed from servo/servo#15536 [ci skip]

jdm mentioned this pull request Apr 17, 2017

Implement setter for document.domain web-platform-tests/wpt#5577

Merged

servo / servo

Implement setter for document.domain #15536

Implement setter for document.domain #15536

asajeffrey commented Feb 13, 2017 •

edited by larsbergstrom

highfive commented Feb 13, 2017

highfive commented Feb 13, 2017

asajeffrey commented Feb 13, 2017

asajeffrey commented Feb 15, 2017

bors-servo commented Feb 18, 2017

bors-servo commented Feb 23, 2017

asajeffrey commented Feb 27, 2017

asajeffrey commented Feb 27, 2017

asajeffrey commented Feb 27, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

asajeffrey commented Mar 2, 2017

asajeffrey commented Mar 8, 2017

asajeffrey commented Mar 14, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nox commented Mar 15, 2017

bors-servo commented Mar 15, 2017

bors-servo commented Mar 15, 2017

This comment has been minimized.

This comment has been minimized.

nox commented Mar 15, 2017

nox commented Mar 15, 2017

asajeffrey commented Mar 15, 2017

asajeffrey commented Mar 15, 2017

bors-servo commented Mar 15, 2017

bors-servo commented Mar 15, 2017

bors-servo commented Mar 15, 2017

servo / servo

Join GitHub today

Implement setter for document.domain #15536

Implement setter for document.domain #15536

Conversation

asajeffrey commented Feb 13, 2017 • edited by larsbergstrom

highfive commented Feb 13, 2017

highfive commented Feb 13, 2017

asajeffrey commented Feb 13, 2017

asajeffrey commented Feb 15, 2017

bors-servo commented Feb 18, 2017

bors-servo commented Feb 23, 2017

asajeffrey commented Feb 27, 2017

asajeffrey commented Feb 27, 2017

asajeffrey commented Feb 27, 2017

This comment has been minimized.

Ms2ger Mar 2, 2017

This comment has been minimized.

asajeffrey Mar 2, 2017

This comment has been minimized.

asajeffrey Mar 2, 2017

asajeffrey commented Mar 2, 2017

asajeffrey commented Mar 8, 2017

asajeffrey commented Mar 14, 2017

This comment has been minimized.

nox Mar 15, 2017

This comment has been minimized.

nox Mar 15, 2017

This comment has been minimized.

nox Mar 15, 2017

nox commented Mar 15, 2017

bors-servo commented Mar 15, 2017

bors-servo commented Mar 15, 2017

This comment has been minimized.

nox Mar 15, 2017

This comment has been minimized.

nox Mar 15, 2017 • edited

nox commented Mar 15, 2017

nox commented Mar 15, 2017

asajeffrey commented Mar 15, 2017

asajeffrey commented Mar 15, 2017

bors-servo commented Mar 15, 2017

bors-servo commented Mar 15, 2017

bors-servo commented Mar 15, 2017

asajeffrey commented Feb 13, 2017 •

edited by larsbergstrom

nox Mar 15, 2017 •

edited