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

Set referrer policy better for <iframe srcdoc> documents #1559

Merged
merged 1 commit into from Aug 11, 2016

Conversation

4 participants
@domenic
Copy link
Member

commented Jul 14, 2016

See w3c/webappsec-referrer-policy#31. Such
documents should not inherit the referrer policy from their parent
document. Instead, we should define the referrer policy for a browsing
context environment settings object to crawl upward when it encounters
iframe srcdoc documents, just like the referrer policy spec already does
for referrers. (It turns out we never actually defined the referrer
policy for browsing context environment settings objects previously;
oops!)

This affects the referrer policy used when making requests from inside
an iframe srcdoc document.

Closes w3c/webappsec-referrer-policy#31. /cc @bzbarsky @estark37.

Set referrer policy better for <iframe srcdoc> documents
See w3c/webappsec-referrer-policy#31. Such
documents should not inherit the referrer policy from their parent
document. Instead, we should define the referrer policy for a browsing
context environment settings object to crawl upward when it encounters
iframe srcdoc documents, just like the referrer policy spec already does
for referrers. (It turns out we never actually defined the referrer
policy for browsing context environment settings objects previously;
oops!)

This affects the referrer policy used when making requests from inside
an iframe srcdoc document.

Closes w3c/webappsec-referrer-policy#31.
@annevk

This comment has been minimized.

Copy link
Member

commented Jul 18, 2016

We should find something better for documents that inherit since I believe we keep forgetting cases and we just copy-and-paste away.

Browsers have some kind of concept of documents that inherit state, which they can reuse for blob, about, data, and srcdoc. And that in turn scales for HTTPS state, referrer policies, referrer, CSP list, origin, cookie URL, base URL, normal URL, ...

@domenic

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2016

That sounds like a good idea, but probably for a follow-up...

@annevk

This comment has been minimized.

Copy link
Member

commented Jul 18, 2016

But this doesn't really solve anything for implementations I think. Since CSP list is not done this way and HTTPS state isn't either. So implementers will continue to get confused as to how this actually works (and I am too, since it's still unclear to me what the actual setup implementations have is).

@domenic

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2016

This is a strict improvement over the current spec in a few ways:

  • It adds the missing definition of referrer policy for browsing context settings objects. This needs to be added no matter what.
  • It removes a header that overrides Referrer Policy's logic for looking upward to determine what policy to use, thus aligning with implementations and with Referrer Policy's intention.

It may not be factored in a way that you or implementations find very understandable, but it is strictly more correct. Refactoring in that way is outside my current capabilities, so I think we should go this route for now.

@annevk

This comment has been minimized.

Copy link
Member

commented Jul 18, 2016

Mkay, I'll let someone else sign-off since I'm not sure I quite understand.

@estark37

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2016

Ok, just checking I understand: it used to be the case that when creating a srcdoc document, we gave it a Referrer-Policy header from its parent's referrer policy. Now we define a browsing context environment settings object's referrer policy by walking upwards until we reach a non-srcdoc document. Fetch uses the browsing context environment settings object's referrer policy (which walks upwards as needed) when setting the referrer policy for a request that doesn't already have a policy set. Is that correct? If so, sounds reasonable to me.

@estark37 estark37 referenced this pull request Aug 11, 2016

Closed

Update web-platform-tests #57

4 of 4 tasks complete
@domenic

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2016

Yep, that's exactly it. Thanks!

@domenic domenic merged commit 5d7c532 into master Aug 11, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@domenic domenic deleted the iframe-srcdoc-referrerpolicy branch Aug 11, 2016

@estark37

This comment has been minimized.

Copy link
Contributor

commented Oct 5, 2016

I was just thinking about this again and wondering if iframe srcdocs should be allowed to have their own referrer policies set from elements?

That is, if I have

<iframe srcdoc="<meta name='referrer' content='no-referrer'><img src=http://example.com/blah.gif"></iframe>

I'd expect the img to be loaded with a referrer policy of no-referrer regardless of what the parent document's referrer policy is.

If that's the behavior we want, I can send a PR so that we only walk up the tree when we're on a srcdoc document that has the default policy (empty string).

@domenic

This comment has been minimized.

Copy link
Member Author

commented Oct 5, 2016

That proposed fix sounds good. I'll let others comment on whether that's the desired semantics, but it sounds reasonable from where I'm sitting.

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

commented Oct 5, 2016

That seems reasonable, yes.

dstockwell pushed a commit to dstockwell/chromium that referenced this pull request Oct 6, 2016

estark Commit bot
Walk up frame tree for srcdoc referrer policies
When deciding the referrer policy for a srcdoc document, walk up the
frame tree until we find a non-srcdoc document OR a srcdoc document with
its own policy set via a meta element.

This implements the algorithm defined in
https://html.spec.whatwg.org/multipage/browsers.html#set-up-a-browsing-context-environment-settings-object. However,
the spec'ed algorithm has to be adjusted per
whatwg/html#1559 (comment) to
account for meta elements in srcdoc documents (which this CL
implements).

BUG=653034,637007

Review-Url: https://codereview.chromium.org/2400443004
Cr-Commit-Position: refs/heads/master@{#423538}

estark37 added a commit to estark37/html that referenced this pull request Oct 6, 2016

Honor srcdoc document referrer policies when set
If a srcdoc document contains a meta element with a referrer policy, we
should use that as the document's referrer policy instead of walking up
the tree to find the first non-srcdoc document's referrer policy.

(As discussed in whatwg#1559 (comment))

estark37 added a commit to estark37/html that referenced this pull request Oct 6, 2016

Honor srcdoc document referrer policies when set
If a srcdoc document contains a meta element with a referrer policy, we
should use that as the document's referrer policy instead of walking up
the tree to find the first non-srcdoc document's referrer policy.

(As discussed in whatwg#1559 (comment))

domenic added a commit that referenced this pull request Nov 23, 2016

Honor srcdoc document referrer policies when set
If a srcdoc document contains a meta element with a referrer policy, we
should use that as the document's referrer policy instead of walking up
the tree to find the first non-srcdoc document's referrer policy.

(As discussed in #1559 (comment))

qtprojectorg pushed a commit to qt/qtwebengine-chromium that referenced this pull request Dec 6, 2016

estark Allan Sandfeld Jensen
[Backport] Walk up frame tree for srcdoc referrer policies
When deciding the referrer policy for a srcdoc document, walk up the
frame tree until we find a non-srcdoc document OR a srcdoc document with
its own policy set via a meta element.

This implements the algorithm defined in
https://html.spec.whatwg.org/multipage/browsers.html#set-up-a-browsing-context-environment-settings-object. However,
the spec'ed algorithm has to be adjusted per
whatwg/html#1559 (comment) to
account for meta elements in srcdoc documents (which this CL
implements).

BUG=653034,637007

Review-Url: https://codereview.chromium.org/2400443004
Cr-Commit-Position: refs/heads/master@{#423538}
(CVE-2016-9650)

Change-Id: Id30ce25c069d254df0ce3767138958d43b6bc8af
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

qtprojectorg pushed a commit to qt/qtwebengine-chromium that referenced this pull request Mar 16, 2017

estark Allan Sandfeld Jensen
[Backport] Walk up frame tree for srcdoc referrer policies
When deciding the referrer policy for a srcdoc document, walk up the
frame tree until we find a non-srcdoc document OR a srcdoc document with
its own policy set via a meta element.

This implements the algorithm defined in
https://html.spec.whatwg.org/multipage/browsers.html#set-up-a-browsing-context-environment-settings-object. However,
the spec'ed algorithm has to be adjusted per
whatwg/html#1559 (comment) to
account for meta elements in srcdoc documents (which this CL
implements).

BUG=653034,637007

Review-Url: https://codereview.chromium.org/2400443004
Cr-Commit-Position: refs/heads/master@{#423538}
(CVE-2016-9650)

Change-Id: Id30ce25c069d254df0ce3767138958d43b6bc8af
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

alice added a commit to alice/html that referenced this pull request Jan 8, 2019

Honor srcdoc document referrer policies when set
If a srcdoc document contains a meta element with a referrer policy, we
should use that as the document's referrer policy instead of walking up
the tree to find the first non-srcdoc document's referrer policy.

(As discussed in whatwg#1559 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.