Issue 1282 - Don't generate filters conflicting with existing exception rules

lib/filterComposer.js

Index: lib/filterComposer.js
===================================================================
--- a/lib/filterComposer.js
+++ b/lib/filterComposer.js
@@ -15,7 +15,10 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-let {getDecodedHostname, stringifyURL} = require("url");
+let {extractHostFromFrame, stringifyURL, isThirdParty} = require("url");
+let {getKey, isFrameWhitelisted} = require("whitelisting");
+let {defaultMatcher} = require("matcher");
+let {WhitelistFilter} = require("filterClasses");
 
 function escapeChar(chr)
 {
@@ -55,9 +58,18 @@
 }
 exports.quoteCSS = quoteCSS;
 
-function canBlockURL(url)
+function canBlockURL(url, type, page, frame)
 {
-  return url.protocol == "http:" || url.protocol == "https:";
+  if (url.protocol != "http:" && url.protocol != "https:")
+    return false;
+
+  let docDomain = extractHostFromFrame(frame);
+  let filter = defaultMatcher.matchesAny(
+    stringifyURL(url), type, docDomain,
+    isThirdParty(url, docDomain), getKey(page, frame)
+  );
+
+  return !(filter instanceof WhitelistFilter);

[Wladimir Palant, 2015/03/02 20:14:19]

For sake of consistency, it might make sense to move that functionality into
whitelisting.js - e.g. isUrlWhitelisted().

[Sebastian Noack, 2015/03/03 14:29:00]

This function no longer exist. The code has been inlined, and some of it's
variables are also used for other purposes now.

 }
 
 /**
@@ -69,20 +81,23 @@
  * @param {string}   [style]  The element's "style" attribute
  * @param {string[]} classes  The classes given by the element's "class" attribute
  * @param {string[]} urls     The URLs considered when loading the element
- * @param {URL}      baseURL  The URL of the document containing the element
+ * @param {string}   [type]   The request type (will be ignored if there are no URLs)
+ * @param {string}   baseURL  The URL of the document containing the element
+ * @param {Page}     page     The page containing the element
+ * @param {Frame}    frame    The frame containing the element
  *
  * @return {object} An object holding the list of generated filters and
  *                  the list of CSS selectors for the included element
  *                  hiding filters: {filters: [...], selectors: [...]}
  */
-function composeFilters(tagName, id, src, style, classes, urls, baseURL)
+function composeFilters(tagName, id, src, style, classes, urls, type, baseURL, page, frame)
 {

[Wladimir Palant, 2015/03/02 20:14:19]

A call to isFrameWhitelisted(..., "DOCUMENT") is missing here - nothing can be
blocked or hidden in a whitelisted frame.

[Sebastian Noack, 2015/03/03 14:29:00]

Done.

-  // Add a blocking filter for each HTTP(S) URL associated with the element
+  // Add a blocking filter for each URL of the element that can be blocked
   let filters = [];
   for (let url of urls)
   {
     let urlObj = new URL(url, baseURL);
-    if (canBlockURL(urlObj))
+    if (canBlockURL(urlObj, type, page, frame))
     {
       let filter = stringifyURL(urlObj).replace(/^[\w\-]+:\/+(?:www\.)?/, "||");
 
@@ -91,30 +106,33 @@
     }
   }
 
-  // Generate CSS selectors based on the element's "id" and "class" attribute
   let selectors = [];
-  if (id)
-    selectors.push("#" + escapeCSS(id));
-  if (classes.length > 0)
-    selectors.push(classes.map(c => "." + escapeCSS(c)).join(""));
+  if (!isFrameWhitelisted(page, frame, "ELEMHIDE"))
+  {
+    // Generate CSS selectors based on the element's "id" and "class" attribute
+    if (id)
+      selectors.push("#" + escapeCSS(id));
+    if (classes.length > 0)
+      selectors.push(classes.map(c => "." + escapeCSS(c)).join(""));
 
-  // If there is a "src" attribute, specifiying a URL that we can't block,
-  // generate a CSS selector matching the "src" attribute
-  if (src && !canBlockURL(new URL(src, baseURL)))
-    selectors.push(escapeCSS(tagName) + "[src=" + quoteCSS(src) + "]");
+    // If there is a "src" attribute, specifiying a URL that we can't block,
+    // generate a CSS selector matching the "src" attribute
+    if (src && !canBlockURL(new URL(src, baseURL), type, page, frame))
+      selectors.push(escapeCSS(tagName) + "[src=" + quoteCSS(src) + "]");
 
-  // As last resort, if there is a "style" attribute, and we couldn't generate
-  // any filters so far, generate a CSS selector matching the "style" attribute
-  if (style && selectors.length == 0 && filters.length == 0)
-    selectors.push(escapeCSS(tagName) + "[style=" + quoteCSS(style) + "]");
+    // As last resort, if there is a "style" attribute, and we couldn't generate
+    // any filters so far, generate a CSS selector matching the "style" attribute
+    if (style && selectors.length == 0 && filters.length == 0)
+      selectors.push(escapeCSS(tagName) + "[style=" + quoteCSS(style) + "]");
 
-  // Add an element hiding filter for each generated CSS selector
-  if (selectors.length > 0)
-  {
-    let domain = getDecodedHostname(baseURL).replace(/^www\./, "");
+    // Add an element hiding filter for each generated CSS selector
+    if (selectors.length > 0)
+    {
+      let domain = extractHostFromFrame(frame).replace(/^www\./, "");
 
-    for (let selector of selectors)
-      filters.push(domain + "##" + selector);
+      for (let selector of selectors)
+        filters.push(domain + "##" + selector);
+    }
   }
 
   return {filters: filters, selectors: selectors};