1318727 - BroadcastChannel doesn't work with null principal

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P5)

Description

[Olli Pettay [:smaug][[e-mail]]]

data:text/html,<script>var b = new BroadcastChannel("foo") </script>
throws a Gecko internal exception.

As far as I see, it should work and one should be able to communicate with other channels from the same null-principal.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Looks like bent's review comments in bug 966439 were just wrong.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

hmm, Chrome throws "Can't create BroadcastChannel in an opaque origin"

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

But I don't see spec saying anything like that. BroadcastChannel should just work.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

I didn't think null principal subsumed any other principal; event other null principals.  I thought it was supposed to represent unique opaque origins.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

So? You can still create a window which has such null principal and use BroadcastChannel inside it.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Olli Pettay [:smaug] (vacation Nov 19-26) from comment #5)
> So? You can still create a window which has such null principal and use
> BroadcastChannel inside it.

And listen to itself?  I guess I'm trying to understand the use case.  It seems no remote environment could ever be considered same-origin here.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

you can create multiple BroadcastChannels inside a window context and make them communicate.
Or you can add about:blank iframes which inherit the origin from data: page etc.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

This is an edge sure, but this also hints that we are doing something wrong with principals.

Comment 9

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Per comment 8, it's a really edge case and there's no real-world use case for now, so put it in P5 (really low priority).

Comment 10

[Andrea Marchesini [:baku]]

Attachment: bc.patch

Smaug, do you mind to review this patch?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Put it to my review queue, thanks.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8814869 [details] [diff] [review]
bc.patch

>   if (aPrincipal->GetIsNullPrincipal()) {
>-    *aPrincipalInfo = NullPrincipalInfo(BasePrincipal::Cast(aPrincipal)->OriginAttributesRef());
>+    nsCOMPtr<nsIURI> uri;
>+    nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
>+    if (NS_WARN_IF(NS_FAILED(rv))) {
>+      return rv;
>+    }
>+
>+    if (NS_WARN_IF(!uri)) {
>+      return NS_ERROR_FAILURE;
>+    }
>+
>+    nsCString spec;
nsAutoCString


Some test is needed here.
Perhaps easiest is to use sandboxed (allow-scripts) data: -url iframe which then tests broadcastchannel and sends messages to its parent window.
That could be a wpt test.

Please don't land this without a test.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/124b6d766926
BroadcastChannel should support data URL, r=smaug

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bdc2dfc9dcc
BroadcastChannel should support data URL - part 2, r=me

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/124b6d766926
https://hg.mozilla.org/mozilla-central/rev/0bdc2dfc9dcc

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

FWIW, Google landed some invalid BroadcastChannel tests to w3c wpt recently.
But the spec isn't going to change
https://github.com/whatwg/html/issues/1319