Assignee: nobody → mozilla
Created attachment 8664046 [details] [diff] [review] bug_1188642_nsobjectloadingcontent.patch Putting up a WIP patch since this bug is still blocked by others, but the question is: What do we do about NS_CheckContentProcessPolicy in general?
Regarding NS_CheckContentProcessPolicy, I would say that we can either leave the checks as-is, or try to move them into the channel. I don't know if the checks are called at a consistent enough place that it makes sense to move into the channel. I.e. if the checks are always happening in OnStartRequest or if they always happen before the channel is opened. If they are too inconsistent I would say leave them as-is.
Some old notes I took on ShouldProcess() can be found on line 59 onwards here: https://etherpad.mozilla.org/ContentPolicies I don't fully understand the use case for it and am not convinced that our implementation is sound.
I share Tanvi's concern that our ShouldProcess implementation is probably not sound. All the more reason to try to fix/remove it as a separate effort in separate bugs.
(In reply to Jonas Sicking (:sicking) from comment #4) > I share Tanvi's concern that our ShouldProcess implementation is probably > not sound. All the more reason to try to fix/remove it as a separate effort > in separate bugs. I agree with both of you. What I remember though, addons are still using them, so I think it's just safest if we leave them as-is for now. Once we are done with moving security checks into AsyncOpen2, we can probably start to deprecate them in a separate effort.
Created attachment 8765571 [details] [diff] [review] bug_1188642_asyncopen2_nsobjectloadingcontent.patch Hey Olli, since :bz is on PTO I was wondering if you feel comfortable reviewing a change within nsObjectLoadingContent or if you could suggest someone else who might be willing to review that change. We have *not* converted nsURILoader to use asyncOpen2() yet (see Bug 1182569), so we can't remove all relevant security checks within nsObjectLoadingContent, because as you can see there is a call to uriloader->OpenChannel() within nsObjectLoadingContent(). Hence I would like to keep this changeset as self contained as possible so that security checks are only skipped at the callsite and enforced within .asyncOpen2() if we are in fact hitting that branch where we create a new channel and call asyncOpen2() on it. Happy to answer more questions if that description does not provide sufficient information.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/2bae08c08118 Use channel->ascynOpen2 in dom/base/nsObjectLoadingContent.cpp r=smaug
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.