Closed Bug 1318727 Opened 3 years ago Closed 3 years ago

BroadcastChannel doesn't work with null principal

Categories

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

50 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: smaug, Assigned: baku)

Details

Attachments

(1 file)

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.
Looks like bent's review comments in bug 966439 were just wrong.
hmm, Chrome throws "Can't create BroadcastChannel in an opaque origin"
But I don't see spec saying anything like that. BroadcastChannel should just work.
I didn't think null principal subsumed any other principal; event other null principals.  I thought it was supposed to represent unique opaque origins.
So? You can still create a window which has such null principal and use BroadcastChannel inside it.
(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.
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.
This is an edge sure, but this also hints that we are doing something wrong with principals.
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).
Priority: -- → P5
Attached patch bc.patchSplinter Review
Smaug, do you mind to review this patch?
Assignee: nobody → amarchesini
Flags: needinfo?(bugs)
Put it to my review queue, thanks.
Flags: needinfo?(bugs) → needinfo?(amarchesini)
Flags: needinfo?(amarchesini)
Attachment #8814869 - Flags: review?(bugs)
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.
Attachment #8814869 - Flags: review?(bugs) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/124b6d766926
BroadcastChannel should support data URL, r=smaug
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bdc2dfc9dcc
BroadcastChannel should support data URL - part 2, r=me
https://hg.mozilla.org/mozilla-central/rev/124b6d766926
https://hg.mozilla.org/mozilla-central/rev/0bdc2dfc9dcc
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
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
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.