BroadcastChannel doesn't work with null principal

RESOLVED FIXED in Firefox 53



a year ago
a year ago


(Reporter: smaug, Assigned: baku)


50 Branch

Firefox Tracking Flags

(firefox53 fixed)



(1 attachment)



a year ago
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

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

Comment 2

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

Comment 3

a year ago
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.

Comment 5

a year ago
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.

Comment 7

a year ago
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

a year ago
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

Comment 10

a year ago
Created attachment 8814869 [details] [diff] [review]

Smaug, do you mind to review this patch?
Assignee: nobody → amarchesini
Flags: needinfo?(bugs)

Comment 11

a year ago
Put it to my review queue, thanks.
Flags: needinfo?(bugs) → needinfo?(amarchesini)


a year ago
Flags: needinfo?(amarchesini)
Attachment #8814869 - Flags: review?(bugs)

Comment 12

a year ago
Comment on attachment 8814869 [details] [diff] [review]

>   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;

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+

Comment 13

a year ago
Pushed by
BroadcastChannel should support data URL, r=smaug

Comment 14

a year ago
Pushed by
BroadcastChannel should support data URL - part 2, r=me

Comment 15

a year ago
Last Resolved: a year ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Comment 16

a year ago
FWIW, Google landed some invalid BroadcastChannel tests to w3c wpt recently.
But the spec isn't going to change
You need to log in before you can comment on or make changes to this bug.