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

Status: NEW

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

Description

[Ben Kelly [:bkelly, not reviewing]]

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.

Comment 1

[Christoph Kerschbaumer [: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

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

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?

Comment 3

[Boris Zbarsky [:bzbarsky]]

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...)

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

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?

Comment 5

[Boris Zbarsky [:bzbarsky]]

> do you mean it can be nullptr or a null principal?

Both.

Comment 6

[Andreas Farre [:farre]]

Ben, is there anything more to be done here?

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Yea, we probably want to do comment 4 and provide a docshell loadinfo in this case.  Its not critical, though.