Repository navigation
Which origins to use in postMessage? #1542
Description
This is part of #1431 and #1430.
postMessage current uses, per spec:
- the entry settings object is used for an origin check if targetOrigin is "/"
- the incumbent settings object to determine the
originandsourceattributes of the resultingMessageEvent.
I'm having a hard time with black-box testing of these cases, so I'm trying to fall back to code inspection.
Code inspection of Blink reveals that Blink uses the current settings object for both of these: https://chromium.googlesource.com/chromium/src/+/540e575dc43e718821bc4ac682735a9631e33c1f/third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp#176
WebKit uses "callerDOMWindow": https://github.com/WebKit/webkit/blob/152410da081c1a638a3c2b8c0f55b5e726fffd15/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp#L552. I'm not sure what this is supposed to be exactly.
Gecko appears to follow the current spec and use both incumbent and entry: http://searchfox.org/mozilla-central/rev/c44d0b1673fef5e0e2e19fa82d6780a74c186151/dom/base/nsGlobalWindow.cpp#8255, although in an offline email @bzbarsky said
I don't see why entry settings is any better than current settings here, really, assuming the current settings bits get sorted out in such a way that we can use current settings for the source origin here.
With this info in hand I'd say the simplest thing to do here is to converge on current for both. @bzbarsky, do you agree? When I quoted you above you were only referring to the "/" thing, but having them be inconsistent seems bad...
This feels like a classic case of enough browsers disagreeing so that we get to just pick the behavior we think is most reasonable.
Activity
bzbarsky commentedon Jul 13, 2016
If Bobby is fine with using current for the
origin(that is, if the variousdocument.domainand cross-origin-objects issues have been resolved to make that do some approximation of the right thing), then that sounds fine.It's not clear to me to what extent UAs implement the currently-specced cross-origin-objects setup, and use of current here would definitely depend on that setup being faithfully enough implemented; otherwise you would get security bugs. Might be worth calling that out in the spec.
bzbarsky commentedon Jul 13, 2016
//cc @bholley
bholley commentedon Jul 19, 2016
Using current for source is pretty bizarre, no? It would mean that same-origin postMessage would always see the source as the receiver of the message, rather than the sender.
If Blink really does set source = current that's strong evidence of web-compatibility, but it seems pretty surprising and we might as well avoid it if we can. I wouldn't keep the incumbent concept around just for this, but if we need it for referrer anyway, I think we should use incumbent here.
domenic commentedon Jul 19, 2016
Ah, interesting. This probably works in Chrome because Chrome's window properties creation is currently broken; it creates realm-appropriate representations for all windows, instead of only creating realm-appropriate representations for cross-origin windows. I wonder what it uses to determine the "caller" in that case, and if it matches incumbent. (My guess is that it does except in edge cases.)
I can do some more black-box testing (since
sourceshould actually be pretty easy to test), but unless I find surprising results I guess it makes sense to stick with incumbent here.You're OK with switching to current for the "targetOrigin is is /" case? Or maybe we should switch to incumbent there; I imagine that's (perhaps approximately) what WebKit is doing with its callerDOMWindow, and in effect what Chrome is doing with its inappropriate extra realm-appropriate wrappers. In general both Chrome and WebKit kind of assume there's a single window which gets passed along through the algorithms and is used for all the checks, and I can't blame them; that does seem like a more reasonable model.
bholley commentedon Jul 19, 2016
If we're using incumbent for source we might as well use it for everything else here, so that the implementation can just get one thing and pass it around, which reduces the chances of bugs.
We should definitely write WPT tests for this stuff to preserve the blackbox testing work that's happening. :-)
Use only the incumbent global in postMessage
Use only the incumbent global in postMessage
Use only the incumbent global in postMessage
Use only the incumbent global in postMessage