Issue 5184 - Support Firefox-specific webRequest types

lib/requestBlocker.js

Index: lib/requestBlocker.js
===================================================================
--- a/lib/requestBlocker.js
+++ b/lib/requestBlocker.js
@@ -27,27 +27,44 @@
 const {checkWhitelisted, getKey} = require("whitelisting");
 const {stringifyURL, extractHostFromFrame, isThirdParty} = require("url");
 const {port} = require("messaging");
 const devtools = require("devtools");
 
 // Chrome can't distinguish between OBJECT_SUBREQUEST and OBJECT requests.
 RegExpFilter.typeMap.OBJECT_SUBREQUEST = RegExpFilter.typeMap.OBJECT;

[Sebastian Noack, 2017/05/18 12:44:24]

Unrelated, but note that this is also true for Firefox WebExtensions. Mind
updating the comment above while doing changes here?

Eventually, these types should be aliased in the definition in adblockpluscore
as there is no platform that has this distinction anymore.

 
+let resourceTypeMappings = new Map([

[Sebastian Noack, 2017/05/18 12:44:24]

Nit: This is a mapping of resource types, so IMHO the variable name should be
resourceTypes or resourceTypeMapping. The former would also be consistent with
the mapping below.

[Manish Jethani, 2017/05/18 20:52:04]

resourceTypes implies all resource types, but I've made it resourceTypeMapping
now.

+  ["beacon", "PING"],
+  ["imageset", "IMAGE"],
+  ["sub_frame", "SUBDOCUMENT"],
+  ["web_manifest", "OTHER"],

[Sebastian Noack, 2017/05/18 12:44:24]

I think we shouldn't hard-code the types we treat as OTHER, but rather handle
each unknown type as OTHER, as these might vary on different platforms, and new
types might be introduced in the future.

[Manish Jethani, 2017/05/18 20:52:04]

My reasoning was that we shouldn't block any resource types that we don't
understand, but I see your point. I've made the change.

I should say that Firefox has a resource type called "csp_report", which is a
request the browser sends to a URL of your choice when there's a content
security policy violation on your page. I don't know if the "other" type of
filters should be able to block these too.

Background (see "report-uri"):

https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP

[Sebastian Noack, 2017/05/19 11:30:46]

Sorry, I forgot to reply to this, since you addressed my comment anyway.

I think it is preferable to block request types we don't know (if they match any
filter, when considered of the type OTHER). Usually, new request types existed
before but were originally reported as OTHER, or are still in some browsers. In
the past there hasn't been a single instance of new request type that would have
made sense to completely ignore.

The csp_report request type might be a little special though. After all this is
a security features, and if websites are no longer notified about CSP violations
that might be bad. But then again, this feature could potentially still be
misused to track users, so being able to block it might be a good thing.

However, there is already one request type which we deliberatly ignore, i.e.
"main_frame". If we decide to do the same for "csp_report", we should do so in
the same way, i.e. by bailing out in webRequest.onBeforeRequest handler.

+  ["xml_dtd", "OTHER"],
+  ["xslt", "OTHER"]
+]);
+
+let typeMasks = new Map(
+  Object.keys(chrome.webRequest.ResourceType)
+  .map(typeKey => chrome.webRequest.ResourceType[typeKey])
+  .map(type => [type, RegExpFilter.typeMap[resourceTypeMappings.get(type)] ||
+                      RegExpFilter.typeMap[type.toUpperCase()] || 0])

[Sebastian Noack, 2017/05/18 12:44:24]

Please use RegExpFilter.typeMap.OTHER, rather than hard-coding 0.

[Manish Jethani, 2017/05/18 20:52:04]

Done.

+);
+
 function onBeforeRequestAsync(page, url, type, docDomain,
                               thirdParty, sitekey,
                               specificOnly, filter)
 {
   if (filter)
     FilterNotifier.emit("filter.hitCount", filter, 0, 0, page);
 
   if (devtools)
   {
     devtools.logRequest(
-      page, url, type, docDomain,
+      page, url,
+      resourceTypeMappings.get(type) || type.toUpperCase(), docDomain,
       thirdParty, sitekey,
       specificOnly, filter
     );
   }
 }
 
 ext.webRequest.onBeforeRequest.addListener((url, type, page, frame) =>
 {
@@ -59,17 +76,17 @@
   let thirdParty = isThirdParty(url, docDomain);
   let sitekey = getKey(page, frame);
 
   let specificOnly = !!checkWhitelisted(
     page, frame, RegExpFilter.typeMap.GENERICBLOCK
   );
 
   let filter = defaultMatcher.matchesAny(
-    urlString, RegExpFilter.typeMap[type],
+    urlString, typeMasks.get(type),
     docDomain, thirdParty, sitekey, specificOnly
   );
 
   setTimeout(onBeforeRequestAsync, 0, page, urlString,
                                       type, docDomain,
                                       thirdParty, sitekey,
                                       specificOnly, filter);