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

document.domain getter without browsing context #2697

Closed
tkent-google opened this issue May 19, 2017 · 10 comments · Fixed by #3742
Closed

document.domain getter without browsing context #2697

tkent-google opened this issue May 19, 2017 · 10 comments · Fixed by #3742

Comments

@tkent-google
Copy link
Contributor

Related WPT test: http://w3c-test.org/html/browsers/origin/relaxing-the-same-origin-restriction/document_domain.html

https://html.spec.whatwg.org/multipage/browsers.html#concept-document-bc

Note: ... A Document created using an API such as createDocument() never has a browsing context.

https://html.spec.whatwg.org/multipage/browsers.html#dom-document-domain

  1. If this Document object does not have a browsing context, then return the empty string.

So such Document instances should return the empty string for domain getter. However it seems all major browsers agree that such Document instances return origin's domain.

  Safari Firefox Edge Chrome
domain of DOMImplementation.createHTMLDocument origin's domain origin's domain origin's domain origin's domain
domain of DOMImplementation.createDocument origin's domain No domain attribute origin's domain origin's domain
domain of new Document() origin's domain No domain attribute No Document constructor No Document constructor
@annevk
Copy link
Member

annevk commented May 19, 2017

See also #1794. I guess making it reflect the origin's domain seems okay.

@domenic
Copy link
Member

domenic commented Jun 8, 2017

I looked into this but I am not sure what the right fix is. In particular when you say "the origin's domain", presumably you don't mean the domain of the origin as defined in https://html.spec.whatwg.org/#concept-origin, since for documents without a browsing context that's a unique opaque origin. Probably you mean the origin of the document's relevant settings object?

This, combined with #1794, implies to me that maybe such documents should actually just inherit the origin of their parent. (As opposed to just changing document.domain's logic.) In fact maybe documents shouldn't even have origins, but instead settings objects should, with all the places that currently refer to the document's origin going to the settings object instead. But that has pretty widespread implications, and I don't know them all.

As such I think it's best to wait for @annevk to get back before we tackle this :-/.

@annevk
Copy link
Member

annevk commented Jun 28, 2017

I was thinking https://html.spec.whatwg.org/#concept-origin-effective-domain (and then serialized appropriately).

@annevk
Copy link
Member

annevk commented Jun 6, 2018

@domenic I think you're correct that we should remove document's origin concept. We only ever make security decisions based on the global's origin. whatwg/dom#410 is also about that.

The current browsing context restriction stems from 7eba67e and 7764ebc which were made without sufficient research. It seems we should be able to remove that for the getter at least.

For the setter the situation seems similar, except that Safari did adopt throwing for documents without a browsing context, as far as I can tell.

cc @bzbarsky

@annevk
Copy link
Member

annevk commented Jun 6, 2018

I suspect @cdumez et al would be okay with removing that restriction again, as it doesn't really serve anything if we look at the relevant settings object's origin. I'll look into updating the specification.

annevk added a commit that referenced this issue Jun 6, 2018
Tests: XXX Need to test the sandboxing restriction properly as at least judging from Chrome's source code that's totally local to the document. So either all documents clone that over somehow from the responsible document, or something is amiss.

Fixes #1794 and fixes #2697.
@bzbarsky
Copy link
Contributor

bzbarsky commented Jun 6, 2018

So in implementation terms....

In Gecko, the global and the document both have an origin attached, for various reasons, but we keep them synchronized. I guess this is the model the spec has too, kind of, via delegating to the associated document.

If you store the origin on the settings object, you need to update it when the settings object's document changes (the initial about:blank case). And then you need to decide whether non-browsing context documents snapshot or alias. Specifically, consider this testcase:

<iframe src="foo.html"></iframe> <!-- same-origin -->
<script>
  var doc = frames[0].document.implementation.createHTMLDocument();
</script>

doc is coming from a window that is aliasing our origin right now (because the load has not happened yet). When the load finishes, that window will have the foo.html origin; in particular it will no longer alias ours. If document.domain now gets set on us, or on the foo.html document, which one, if any, affects the value of doc.domain?

I guess similar questions can be asked about document.open, but I don't recall the spec model for it well enough right now, nor the actual UA implementations, well enough to raise a specific problem.

There may also be similar questions around javascript:; again I don't recall enough about the spec model to tell for sure.

@annevk
Copy link
Member

annevk commented Jun 6, 2018

Okay, it seems that's how it works in Chrome too.

It seems that for the setter Chrome will validate any input, but only update the origin if there's a browsing context. Safari just throws early if there's no browsing context. So maybe the current specification of throwing when there's no browsing context is what we should try to align on, as that seems like a much more reasonable model.

@bzbarsky
Copy link
Contributor

bzbarsky commented Jun 6, 2018

I think that seems reasonable. So throw on write and do something (tbd) on read?

annevk added a commit that referenced this issue Jun 6, 2018
@annevk
Copy link
Member

annevk commented Jun 6, 2018

@bzbarsky on read you'd return document's origin (we'd keep the concept). What exactly that origin is for various documents is mostly defined, but when it's not matching implementations we'll adjust the specification(s).

@bzbarsky
Copy link
Contributor

bzbarsky commented Jun 6, 2018

Makes sense.

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