As far as I can tell, nsISiteSecurityService.shouldIgnoreHeaders isn't used anymore (somewhere along the line of refactoring to get HPKP out the door, we pulled that functionality directly into the processing of headers but never removed the unused/duplicated code). This basically involves: * removing the declaration in nsISiteSecurityService * (including updating the uuid) * removing the implementation in nsSiteSecurityService

I am new to open source and I am willing to work on this bug if the bug is unsolved. please give me some work to start from.

Comment on attachment 8540020 [details] [diff] [review] bug-1111848.diff Review of attachment 8540020 [details] [diff] [review]: ----------------------------------------------------------------- Great! r=me with removing the (now) unnecessary #include (see comment). ::: security/manager/boot/src/nsSiteSecurityService.cpp @@ -972,5 @@ > - bool* aResult) > -{ > - nsresult rv; > - bool tlsIsBroken = false; > - nsCOMPtr<nsISSLStatusProvider> sslprov = do_QueryInterface(aSecurityInfo); I believe you can also remove the #include for nsISSLStatusProvider.h now (but not nsISSLStatus).

(In reply to kunal jain from comment #2) > I am new to open source and I am willing to work on this bug if the bug is > unsolved. > please give me some work to start from. Thanks, Kunal, but it looks like David has already done the necessary work. If you're interested, bug 1077954 is another [good first bug] related to security.

Sid - just a heads-up: this might affect ForceTLS (the implementation of nsISiteSecurityService now checks the trustworthiness of the nsISSLStatus of the connection that's attempting to set the header, so the separate ShouldIgnoreHeaders function is no longer necessary).

Thanks for the quick turnaround David! I've updated the patch to remove the nsISSLStatusProvider.h include.

Comment on attachment 8540549 [details] [diff] [review] Remove nsISSLStatusProvider.h include Review of attachment 8540549 [details] [diff] [review]: ----------------------------------------------------------------- Great!

Thanks! How do I go about getting the patch committed? From the docs, it seems it needs to be submitted to the try server first? (I don't have access to the try server, so can't push the patch myself).

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=69a0e041f373 I don't think it *really* needs a try run since it's code removal, but can't hurt. If this looks good, then you can set the checkin-needed keyword on this bug and a kind sheriff will check it in (or I'll check back and maybe check it in for you).

Yep, try looked good. Great job! Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/178d18db618d

Adding addon-compat just to document it, but it looks like only Sid's add-on is affected :)