Intent to Implement and Ship: Return null from contentDocument and getSVGDocument() on cross-origin access

352 views
Skip to first unread message

Daniel Cheng

unread,
Feb 6, 2018, 2:09:34 PM2/6/18
to blink-dev
dch...@chromium.org https://html.spec.whatwg.org/multipage/iframe-embed-object.html#dom-iframe-contentdocument Change the contentDocument attribute and the getSVGDocument() method of HTMLEmbedElement, HTMLIframeElement, and HTMLObjectElement to return null on cross-origin access, rather than throwing an exception.
1. It aligns the behavior of Chrome with Firefox and the HTML standard.
2. The behavior as implemented is a minor info leak. Calling getSVGDocument() on a cross-origin document that *is not* a SVG document will simply return null. Calling getSVGDocument() on a cross-origin document that *is* a SVG document will throw.
3. It reduces the implementation complexity of out-of-process iframes: currently, information about the remote document type isn't replicated. Preserving the existing behavior would require replicating the document type of a remote contentDocument to its parent frame and updating it when it changes.
Firefox: Shipped Edge: No public signals Safari: No public signals Web developers: No signals
Firefox is the only browser that returns null on cross-origin access for contentDocument and getSVGDocument(). All other browsers currently match Chrome behavior.

That being said, the chance of breakage should be low: Firefox has implemented and shipped this behavior since 2013.
None.
Yes. https://crbug.com/582245 https://www.chromestatus.com/features/5776623743795200
Yes.

Rick Byers

unread,
Feb 6, 2018, 5:30:52 PM2/6/18
to Daniel Cheng, blink-dev
Any chance we have a UseCounter for this exception being thrown?  Even when shipped by Firefox we generally like to have some idea of the upper bound on the fraction of page views that could be broken by a change like this.  Eg. it wouldn't be unprecedented for some major library or site to have code behind a UA string check which depends on the Chrome behavior.  If we don't already have a UseCounter (or any other quantifiable data suggesting the compat risk is indeed low), then perhaps we should add one, do a spot check at the data from canary, and then ship pro-actively but double check the counter sometime during beta?  I agree with your assessment that it seems pretty unlikely that anyone is depending on this, but these sorts of things are notoriously hard to predict so we try to rely on some hard data for all breaking changes ;-)

Also are there existing web-platform-tests?  The current template has a section for this. 

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKq9dwYUGVKHmRmSAiXe52my54z%2BwOEC7ZMP91bt6goXZw%40mail.gmail.com.

Daniel Cheng

unread,
Feb 6, 2018, 5:56:56 PM2/6/18
to Rick Byers, blink-dev
Answers inline.

On Tue, Feb 6, 2018 at 2:30 PM Rick Byers <rby...@chromium.org> wrote:
Any chance we have a UseCounter for this exception being thrown?  Even when shipped by Firefox we generally like to have some idea of the upper bound on the fraction of page views that could be broken by a change like this.  Eg. it wouldn't be unprecedented for some major library or site to have code behind a UA string check which depends on the Chrome behavior.  If we don't already have a UseCounter (or any other quantifiable data suggesting the compat risk is indeed low), then perhaps we should add one, do a spot check at the data from canary, and then ship pro-actively but double check the counter sometime during beta?  I agree with your assessment that it seems pretty unlikely that anyone is depending on this, but these sorts of things are notoriously hard to predict so we try to rely on some hard data for all breaking changes ;-)

Fair enough. We don't have a use counter, since the exception is currently thrown by the generated bindings, but it would be easy to add a counter in Blink itself. Note that this would be a pretty rough use counter though: I don't know of a way to count instances where the exception is thrown *and* caught (which is presumably where any behavior deltas would come from).
 

Also are there existing web-platform-tests?  The current template has a section for this. 

I didn't look extensively, but getSVGDocument() doesn't appear to be covered by any tests. contentDocument looks like it might be indirectly covered by things like frame-ancestors-overrides-xfo.html, which Firefox currently fails.

Daniel

Daniel Bratell

unread,
Feb 6, 2018, 6:23:18 PM2/6/18
to Rick Byers, Daniel Cheng, blink-dev
I'd be fine with tracking this in parallel with implementing and shipping it and revert at signs of higher risk than expected.

i.e. LGTM1.

/Daniel
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKppQS1iXm6QqUAnHz0wSB5j-mrpKGKmwcw8G%2Bmff52mrQ%40mail.gmail.com.



--
/* Opera Software, Linköping, Sweden: CET (UTC+1) */

Rick Byers

unread,
Feb 6, 2018, 8:22:13 PM2/6/18
to Daniel Bratell, Daniel Cheng, blink-dev
On Tue, Feb 6, 2018 at 6:23 PM, Daniel Bratell <bra...@opera.com> wrote:
I'd be fine with tracking this in parallel with implementing and shipping it and revert at signs of higher risk than expected.

i.e. LGTM1.

/Daniel

On Tue, 06 Feb 2018 23:56:38 +0100, Daniel Cheng <dch...@chromium.org> wrote:

Answers inline.

On Tue, Feb 6, 2018 at 2:30 PM Rick Byers <rby...@chromium.org> wrote:
Any chance we have a UseCounter for this exception being thrown?  Even when shipped by Firefox we generally like to have some idea of the upper bound on the fraction of page views that could be broken by a change like this.  Eg. it wouldn't be unprecedented for some major library or site to have code behind a UA string check which depends on the Chrome behavior.  If we don't already have a UseCounter (or any other quantifiable data suggesting the compat risk is indeed low), then perhaps we should add one, do a spot check at the data from canary, and then ship pro-actively but double check the counter sometime during beta?  I agree with your assessment that it seems pretty unlikely that anyone is depending on this, but these sorts of things are notoriously hard to predict so we try to rely on some hard data for all breaking changes ;-)

Fair enough. We don't have a use counter, since the exception is currently thrown by the generated bindings, but it would be easy to add a counter in Blink itself. Note that this would be a pretty rough use counter though: I don't know of a way to count instances where the exception is thrown *and* caught (which is presumably where any behavior deltas would come from).

There could be deltas in the failure mode even without being caught.  Anyway yeah, the UseCounters are always an over-estimate of the risk.  Your intuition is that these exceptions should be quite rare (eg. <0.01% of page views) today - right?  Or do you think it's possible that the exceptions are actually relatively common yet this change is still safe (in which case we won't get any real value out of adding a UseCounter)?
 

Also are there existing web-platform-tests?  The current template has a section for this. 

I didn't look extensively, but getSVGDocument() doesn't appear to be covered by any tests. contentDocument looks like it might be indirectly covered by things like frame-ancestors-overrides-xfo.html, which Firefox currently fails.

Can you please add a simple test when you change this then?  That'll help track the issue in other engines as well.
 

Daniel
 

On Tue, Feb 6, 2018 at 2:09 PM, Daniel Cheng <dch...@chromium.org> wrote:
dch...@chromium.org https://html.spec.whatwg.org/multipage/iframe-embed-object.html#dom-iframe-contentdocument Change the contentDocument attribute and the getSVGDocument() method of HTMLEmbedElement, HTMLIframeElement, and HTMLObjectElement to return null on cross-origin access, rather than throwing an exception.
1. It aligns the behavior of Chrome with Firefox and the HTML standard.
2. The behavior as implemented is a minor info leak. Calling getSVGDocument() on a cross-origin document that *is not* a SVG document will simply return null. Calling getSVGDocument() on a cross-origin document that *is* a SVG document will throw.
3. It reduces the implementation complexity of out-of-process iframes: currently, information about the remote document type isn't replicated. Preserving the existing behavior would require replicating the document type of a remote contentDocument to its parent frame and updating it when it changes.
Firefox: Shipped Edge: No public signals Safari: No public signals Web developers: No signals
Firefox is the only browser that returns null on cross-origin access for contentDocument and getSVGDocument(). All other browsers currently match Chrome behavior.

That being said, the chance of breakage should be low: Firefox has implemented and shipped this behavior since 2013.
None.
Yes. https://crbug.com/582245 https://www.chromestatus.com/features/5776623743795200
Yes.
--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKq9dwYUGVKHmRmSAiXe52my54z%2BwOEC7ZMP91bt6goXZw%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

Chris Harrelson

unread,
Feb 9, 2018, 1:19:29 PM2/9/18
to Rick Byers, Daniel Bratell, Daniel Cheng, blink-dev
LGTM2. I agree it should be ok to ship in parallel with use counting to verify safety, plus WPT testing of the two methods being changed.


To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY8Mt%3D9eicSKsH25G-Ufn6X1%3DMX%3DF_Js1KQcn7OJSc%2Bucg%40mail.gmail.com.

Rick Byers

unread,
Feb 15, 2018, 4:05:56 PM2/15/18
to Chris Harrelson, Daniel Bratell, Daniel Cheng, blink-dev
LGTM3 also assuming we get a WPT added.

Daniel Cheng

unread,
Feb 15, 2018, 7:34:42 PM2/15/18
to Rick Byers, Chris Harrelson, Daniel Bratell, blink-dev
Indeed, I will add WPTs to cover these changes as well as some usage numbers. Thanks!

Daniel

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.



--
/* Opera Software, Linköping, Sweden: CET (UTC+1) */

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
Reply all
Reply to author
Forward
0 new messages