We need to make CSPService implement nsIChannelEventSink so we can get called when channels redirect.

Make CSPService implement nsIChannelEventSink

Fixed some small bit rot.

Comment on attachment 421162 [details] [diff] [review] csp-channel-event-sink-v7.4 - In CSPService::OnChannelRedirect(): + props->GetPropertyAsInterface(NS_LITERAL_STRING("csp.channelPolicy"), + NS_GET_IID(nsISupports), + getter_AddRefs(policyContainer)); Seems like this should use NS_CHANNEL_PROP_CHANNEL_POLICY now that bug 515797 has landed. + if (!policyContainer) + return NS_OK; + + // see if we have a valid nsIChannelPolicy containing CSP and load type + nsCOMPtr<nsIChannelPolicy> channelPolicy(do_QueryInterface(policyContainer)); + if (!channelPolicy) + return NS_OK; I wonder if we should disallow redirects if we for some reason find a non-null channel policy and it does *not* implement nsIChannelPolicy? Seems like that's either a bad bug in our code, or someone's monkeying around with channel policies that we probably don't want to support? If not, we can drop the first of those two if checks. + if (aDecision != 1) { + newChannel->Cancel(NS_BINDING_FAILED); + } + + // if the redirect is permitted, propagate the Content Security Policy + // and load type to the redirecting channel + else { I'd recommend moving the above comment inside the else case to make it much harder for someone to later on inserting code between the if and the else here. If I was writing this I'd write it like this: if (...) { // comments... do stuff } else { // comments... do other stuff } + props->SetPropertyAsInterface(NS_LITERAL_STRING("csp.channelPolicy"), + channelPolicy); Use NS_CHANNEL_PROP_CHANNEL_POLICY here too. r=jst with that looked into.

Addresses jst's review comments. Ready to be checked in.

I'll check this in when the tree is green. FYI, the increased size of the final patch is only due to whitespace fixes.

Comment on attachment 440038 [details] [diff] [review] csp-channel-event-sink-final Carrying forward jst's r+.