Closed Bug 771736 Opened 8 years ago Closed 8 years ago

Bug 767134 breaks some nsIContentPolicy consumers


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

Not set



Tracking Status
firefox16 - ---


(Reporter: devd, Assigned: devd)



(Keywords: addon-compat, regression) added another argument to nsIContentPolicy, the principal of the page that caused the request. Unfortunately, the changes broke RequestPolicy.
Blocks: 767134
Keywords: regression
Summary: Bug 767134 breaks nsIContentPolicy consumers → Bug 767134 breaks some nsIContentPolicy consumers
Assignee: nobody → dev.akhawe+mozilla
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.
@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?
I believe the new behavior is strictly better, fwiw.
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. ;)
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.
Closed: 8 years ago
Resolution: --- → WONTFIX
> @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? 


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...
Blocks: 803765
You need to log in before you can comment on or make changes to this bug.