Issue 3108 - Implement security checks for elemhide registration channel on our end instead of leav…

Issue 3108 - Implement security checks for elemhide registration channel on our end instead of leav…

[Wladimir Palant, 2015-10-15 15:19:03]

Note that this only seems to be solving one part of the problem - good enough
when E10S is switched off. However, in E10S mode I still have issues, looking
into that.

[Wladimir Palant, 2015-10-15 15:56:07]

The issue with E10S is really that it isn't our implementation being queried
there but a compatibility shim. Registering our protocol handler in all
processes would solve that, but that's different from the issue addressed here -
I'll create a separate review for that.

[Thomas Greiner, 2015-10-15 16:04:45]

https://codereview.adblockplus.org/29329154/diff/29329155/lib/elemHideHitRegi...
File lib/elemHideHitRegistration.js (right):

https://codereview.adblockplus.org/29329154/diff/29329155/lib/elemHideHitRegi...
lib/elemHideHitRegistration.js:71: newChannel: function(uri, loadInfo)
Where is `loadInfo` coming from? It's not specified in the `nsIAboutModule`
interface.

[Wladimir Palant, 2015-10-15 17:59:16]

https://codereview.adblockplus.org/29329154/diff/29329155/lib/elemHideHitRegi...
File lib/elemHideHitRegistration.js (right):

https://codereview.adblockplus.org/29329154/diff/29329155/lib/elemHideHitRegi...
lib/elemHideHitRegistration.js:71: newChannel: function(uri, loadInfo)
On 2015/10/15 16:04:44, Thomas Greiner wrote:
> Where is `loadInfo` coming from? It's not specified in the `nsIAboutModule`
> interface.

It is:
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/about/nsIAbout...
(not on MDN of course because nobody thought of setting dev-doc-needed on
https://bugzilla.mozilla.org/show_bug.cgi?id=1067468). This is all part of a
huge refactoring effort under
https://bugzilla.mozilla.org/show_bug.cgi?id=1006868.

It's being passed in by the networking code. Older Firefox versions don't have
that parameter, but these also don't expect us to set nsIChannel.loadInfo - so
passing undefined around is ok here.

[Thomas Greiner, 2015-10-15 18:03:09]

LGTM

https://codereview.adblockplus.org/29329154/diff/29329155/lib/elemHideHitRegi...
File lib/elemHideHitRegistration.js (right):

https://codereview.adblockplus.org/29329154/diff/29329155/lib/elemHideHitRegi...
lib/elemHideHitRegistration.js:71: newChannel: function(uri, loadInfo)
On 2015/10/15 17:59:16, Wladimir Palant wrote:
> On 2015/10/15 16:04:44, Thomas Greiner wrote:
> > Where is `loadInfo` coming from? It's not specified in the `nsIAboutModule`
> > interface.
> 
> It is:
>
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/about/nsIAbout...
> (not on MDN of course because nobody thought of setting dev-doc-needed on
> https://bugzilla.mozilla.org/show_bug.cgi?id=1067468). This is all part of a
> huge refactoring effort under
> https://bugzilla.mozilla.org/show_bug.cgi?id=1006868.
> 
> It's being passed in by the networking code. Older Firefox versions don't have
> that parameter, but these also don't expect us to set nsIChannel.loadInfo - so
> passing undefined around is ok here.

I see, thanks for the details. I shouldn't have relied on MDN in that case.