Issue 5184 - Support Firefox-specific webRequest types

lib/requestBlocker.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 11 matching lines @@
 const {Filter, RegExpFilter, BlockingFilter} = require("filterClasses");
 const {Subscription} = require("subscriptionClasses");
 const {defaultMatcher} = require("matcher");
 const {FilterNotifier} = require("filterNotifier");
 const {Prefs} = require("prefs");
 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.
+// Chrome and Firefox (WebExtensions) can't distinguish between
+// OBJECT_SUBREQUEST and OBJECT requests.
 RegExpFilter.typeMap.OBJECT_SUBREQUEST = RegExpFilter.typeMap.OBJECT;
 
-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"]
-]);
+// Map of content types reported by the browser to the respecitve content types
+// used by Adblock Plus. Other content types are simply mapped to OTHER.
+let resourceTypes = new Map(function*()
+{
+  for (let type in RegExpFilter.typeMap)
+    yield [type.toLowerCase(), type];
 
-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.

-);
+  yield ["sub_frame", "SUBDOCUMENT"];
+
+  // Treat navigator.sendBeacon() the same as <a ping>, it's essentially the
+  // same concept - merely generalized.
+  yield ["beacon", "PING"];
+
+  // Treat <img srcset> and <picture> the same as other images.
+  yield ["imageset", "IMAGE"];
+}());
 
 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,
-      resourceTypeMappings.get(type) || type.toUpperCase(), docDomain,
+      page, url, type, docDomain,
       thirdParty, sitekey,
       specificOnly, filter
     );
   }
 }
 
 ext.webRequest.onBeforeRequest.addListener((url, type, page, frame) =>
 {
   if (checkWhitelisted(page, frame))
     return true;
 
   let urlString = stringifyURL(url);
   let docDomain = extractHostFromFrame(frame);
   let thirdParty = isThirdParty(url, docDomain);
   let sitekey = getKey(page, frame);
 
   let specificOnly = !!checkWhitelisted(
     page, frame, RegExpFilter.typeMap.GENERICBLOCK
   );
 
+  let mappedType = resourceTypes.get(type) || "OTHER";
+
   let filter = defaultMatcher.matchesAny(
-    urlString, typeMasks.get(type),
+    urlString, RegExpFilter.typeMap[mappedType],
     docDomain, thirdParty, sitekey, specificOnly
   );
 
   setTimeout(onBeforeRequestAsync, 0, page, urlString,
-                                      type, docDomain,
+                                      mappedType, docDomain,
                                       thirdParty, sitekey,
                                       specificOnly, filter);
 
   return !(filter instanceof BlockingFilter);
 });
 
 port.on("filters.collapse", (message, sender) =>
 {
   if (checkWhitelisted(sender.page, sender.frame))
     return false;
@@ skipping 79 matching lines @@
   if (msg.requestType in chrome.webRequest.ResourceType)
     return false;
 
   return ext.webRequest.onBeforeRequest._dispatch(
      new URL(msg.url),
      msg.requestType,
      sender.page,
      sender.frame
   ).includes(false);
 });