771736 - Bug 767134 breaks some nsIContentPolicy consumers

Status: RESOLVED WONTFIX

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

Description

[Devdatta Akhawe [:devd]]

https://bugzil.la/767134 added another argument to nsIContentPolicy, the principal of the page that caused the request. Unfortunately, the changes broke RequestPolicy.

Comment 1

[Justin Samuel]

Bug 767134 didn't cause any problems for RequestPolicy due to the new argument. It's recent changes in aRequestOrigin that broke one of RequestPolicy's workarounds for missing information.

How things used to work was that, in shouldLoad(), when RequestPolicy saw aRequestOrigin was moz-nullprincipal, it would use aContext.contentDocument.documentURI as the referrer. That was just a hack to deal with the fact that aRequestOrigin was moz-nullprincipal sometimes when there really should have been an actual referrer there. For URLs entered in the location bar, aRequestOrigin used to be a chrome:// URI (specifically, chrome://browser/content/browser.xul).

As best as I can tell with Fx 16, possibly not completely due to bug 767134, I can now always count on aRequestOrigin being the actual referrer and it will be moz-nullprincipal only when there really isn't a referrer. So, RP's moz-nullprincipal/aContext.contentDocument hack now causes problems because aRequestOrigin is a moz-nullprincipal URI instead of a chrome:// URI when a URL is entered in the location bar.

If I'm wrong about the fact that aRequestOrigin will now only be a moz-nullprincipal URI when there truly isn't a referrer, then my fix for these Fx changes just introduced an RP whitelist bypass. That would make me sad.

Comment 2

[Devdatta Akhawe [:devd]]

@Justin: I made a mistake in my original comment: 767134 did actually make the changes that you are seeing. Looking at your explanation though, I am not sure whether I should go ahead and undo the changes or not. It does seem that the changes have made your code cleaner. Would a change that continues sending chrome:// referers for user navigation be sufficient?

@bz: suggestions?

Comment 3

[Boris Zbarsky [:bzbarsky]]

I believe the new behavior is strictly better, fwiw.

Comment 4

[Justin Samuel]

I like the new behavior perfectly fine as long as my new assumption is valid: if aRequestOrigin is moz-nullprincipal, the request has no referrer. That is, no logical referrer, not "no referrer as far as Firefox could keep track of" which used to be the case when aRequestOrigin was moz-nullprincipal.

The fix for RequestPolicy was easy and some year in the future I'll be able to remove the old, ugly code. ;)

Comment 5

[Devdatta Akhawe [:devd]]

I think we should definitely aim for the invariant "no logical referer if null", and afaik this was the only case where it used to be a problem. But I don't know if there are other places where this invariant is violated. 

@bz: Is it safe to say that if we do find such a case (i.e., where a logical referer exists, but firefox didn't track it), we will fix that? 

Additionally, we should also maintain the invariant that aRequestPrincipal (the new argument) is the logical principal that caused this network request.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> @bz: Is it safe to say that if we do find such a case (i.e., where a logical referer
> exists, but firefox didn't track it), we will fix that? 

Yes.

Though note that our notion of "request principal" and the HTTP referrer can be slightly different.

For example, if a web site has frames A and B and A calls a function in B which sets the location of some window, then the principal is B but the HTTP referrer is A.  As long as all you care about is the origin, it's all the same...