Open Bug 1430348 Opened 2 years ago Updated 10 months ago

Clients.openWindow() navigation request in e10s gets a null principal for nsILoadInfo.principalToInherit

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: bkelly, Unassigned)

References

Details

While trying to validate the principal of the initial ClientInfo after IPC deserialization in bug 1231211 I noticed something odd.  It seems Clients.openWindow() in e10s mode creates a navigation nsIChannel with a null principal principalToInherit.  This seems unexpected and possibly suggests we're not passing the right parameters to nsWindowWatcher::OpenWindow2() here:

https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/dom/clients/manager/ClientOpenWindowUtils.cpp#219

I think probably its due to not passing an explicit triggering principal in a docshell loadinfo.

Christoph, do you have any idea how we might get a null principal for principalToInherit here?

Note, dom/workers/test/serviceworkers/test_openWindow.html will exercise this code.

In bug 1231211 I decided to validate the initial about:blank principal against the loading or triggering principal instead.  This provides the expected principal.
Flags: needinfo?(ckerschb)
(In reply to Ben Kelly [:bkelly] from comment #0)
> Christoph, do you have any idea how we might get a null principal for
> principalToInherit here?

I am not entirely sure about this particular case and before I provide an invalid answer I'll rather forward the ni? to Boris. Generally speaking, it's entirely possible that the principalToInherit is null within the loadInfo [1] even for loads of TYPE_DOCUMENT or TYPE_SUBDOCUMENT. If the inherit flag is set, then the triggeringPrincipal is the principal you are looking for and what you did seems correct.

Boris, can you confirm?

[1] https://dxr.mozilla.org/mozilla-central/source/netwerk/base/nsILoadInfo.idl#302-309
Flags: needinfo?(ckerschb) → needinfo?(bzbarsky)
Its possible I am getting confused.  I was thinking about "inherit the principal" in terms of an initial about:blank iframe inheriting the parent's principal.  Perhaps `principalToInherit` is about something different?
principalToInherit is the principal to be inherited if we're inheriting (like for about:blank).  But it's allowed to be null, in which case the principal to be inherited is the triggering principal.

Whether it should be null in your case or not depends on what behavior you want.  In this particular case, there is no parent window and no docshell loadinfo being provided, so there really is nothing to inherit.  Should there be inheriting going on here, and if so from what?

(I would expect the loading and triggering principals to not be terribly useful here either, fwiw, because this load is being done with no explicit trigger or loader...)
Flags: needinfo?(bzbarsky)
It should probably be providing a docshell loadinfo with a trigger principal set to the principal used to call Clients.openWindow().

When you say "it can be null", do you mean it can be nullptr or a null principal?
Flags: needinfo?(bzbarsky)
> do you mean it can be nullptr or a null principal?

Both.
Flags: needinfo?(bzbarsky)
Ben, is there anything more to be done here?
Flags: needinfo?(bkelly)
Yea, we probably want to do comment 4 and provide a docshell loadinfo in this case.  Its not critical, though.
Flags: needinfo?(bkelly)
Priority: -- → P3
No longer blocks: ServiceWorkers-e10s
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.