Issue 443 - Fix blocking pop-ups after redirect

lib/contentPolicy.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 /**
  * @fileOverview Content policy implementation, responsible for blocking things.
  */
 
-Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Services.jsm");
+"use strict";
+
+let {XPCOMUtils} = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
+let {Services} = Cu.import("resource://gre/modules/Services.jsm", {});
 
 let {Utils} = require("utils");
 let {Prefs} = require("prefs");
 let {FilterStorage} = require("filterStorage");
 let {BlockingFilter, WhitelistFilter, RegExpFilter} = require("filterClasses");
 let {defaultMatcher} = require("matcher");
 let {objectMouseEventHander} = require("objectTabs");
 let {RequestNotifier} = require("requestNotifier");
 let {ElemHide} = require("elemHide");
 
@@ skipping 45 matching lines @@
   localizedDescr: {},
 
   /**
    * Lists the non-visual content types.
    * @type Object
    */
   nonVisual: {},
 
   /**
    * Map containing all schemes that should be ignored by content policy.
-   * @type Object
-   */
-  whitelistSchemes: {},
+   * @type Set
+   */
+  whitelistSchemes: new Set(),
 
   /**
    * Called on module startup, initializes various exported properties.
    */
   init: function()
   {
     // type constant by type description and type description by type constant
     let iface = Ci.nsIContentPolicy;
     for (let typeName of contentTypes)
     {
@@ skipping 25 matching lines @@
     this.type.POPUP = 0xFFFE;
     this.typeDescr[0xFFFE] = "POPUP";
     this.localizedDescr[0xFFFE] = Utils.getString("type_label_popup");
     this.typeMask[0xFFFE] = RegExpFilter.typeMap.POPUP;
 
     for (let type of nonVisualTypes)
       this.nonVisual[this.type[type]] = true;
 
     // whitelisted URL schemes
     for (let scheme of Prefs.whitelistschemes.toLowerCase().split(" "))
-      this.whitelistSchemes[scheme] = true;
+      this.whitelistSchemes.add(scheme);
 
     // Generate class identifier used to collapse node and register corresponding
     // stylesheet.
     let offset = "a".charCodeAt(0);
     for (let i = 0; i < 20; i++)
       collapsedClass +=  String.fromCharCode(offset + Math.random() * 26);
 
     let collapseStyle = Services.io.newURI("data:text/css," +
         encodeURIComponent("." + collapsedClass +
         "{-moz-binding: url(chrome://global/content/bindings/general.xml#foobarbazdummy) !important;}"), null, null);
     Utils.styleService.loadAndRegisterSheet(collapseStyle, Ci.nsIStyleSheetService.USER_SHEET);
-    onShutdown.add(function()
+    onShutdown.add(() =>
     {
       Utils.styleService.unregisterSheet(collapseStyle, Ci.nsIStyleSheetService.USER_SHEET);
     });
   },
 
   /**
    * Checks whether a node should be blocked, hides it if necessary
    * @param wnd {nsIDOMWindow}
    * @param node {nsIDOMElement}
    * @param contentType {String}
@@ skipping 138 matching lines @@
     return !match || match instanceof WhitelistFilter;
   },
 
   /**
    * Checks whether the location's scheme is blockable.
    * @param location  {nsIURI}
    * @return {Boolean}
    */
   isBlockableScheme: function(location)
   {
-    return !(location.scheme in Policy.whitelistSchemes);
+    return !this.whitelistSchemes.has(location.scheme);
   },
 
   /**
    * Checks whether a page is whitelisted.
    * @param {String} url
    * @param {String} [parentUrl] location of the parent page
    * @param {String} [sitekey] public key provided on the page
    * @return {Filter} filter that matched the URL or null if not whitelisted
    */
   isWhitelisted: function(url, parentUrl, sitekey)
   {
     if (!url)
       return null;
 
     // Do not apply exception rules to schemes on our whitelistschemes list.
     let match = /^([\w\-]+):/.exec(url);
-    if (match && match[1] in Policy.whitelistSchemes)
+    if (match && this.whitelistSchemes.has(match[1]))
       return null;
 
     if (!parentUrl)
       parentUrl = url;
 
     // Ignore fragment identifier
     let index = url.indexOf("#");
     if (index >= 0)
       url = url.substring(0, index);
 
     let result = defaultMatcher.matchesAny(url, RegExpFilter.typeMap.DOCUMENT, getHostname(parentUrl), false, sitekey);
     return (result instanceof WhitelistFilter ? result : null);
   },
 
   /**
    * Checks whether the page loaded in a window is whitelisted for indication in the UI.
    * @param wnd {nsIDOMWindow}
    * @return {Filter} matching exception rule or null if not whitelisted
    */
   isWindowWhitelisted: function(wnd)
   {
-    return Policy.isWhitelisted(getWindowLocation(wnd));
+    return this.isWhitelisted(getWindowLocation(wnd));
   },
 
   /**
    * Asynchronously re-checks filters for given nodes.
    * @param {Node[]} nodes
    * @param {RequestEntry} entry
    */
   refilterNodes: function(nodes, entry)
   {
     // Ignore nodes that have been blocked already
@@ skipping 22 matching lines @@
    */
   init: function()
   {
     let registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
     registrar.registerFactory(this.classID, this.classDescription, this.contractID, this);
 
     let catMan = Utils.categoryManager;
     for (let category of this.xpcom_categories)
       catMan.addCategoryEntry(category, this.contractID, this.contractID, false, true);
 
-    // http-on-opening-request is new in Gecko 18, http-on-modify-request can
-    // be used in earlier releases.
-    let httpTopic = "http-on-opening-request";
-    if (Services.vc.compare(Utils.platformVersion, "18.0") < 0)
-      httpTopic = "http-on-modify-request";
-
-    Services.obs.addObserver(this, httpTopic, true);
     Services.obs.addObserver(this, "content-document-global-created", true);
 
-    onShutdown.add(function()
-    {
-      // Our category observers should be removed before changing category
-      // memberships, just in case.
-      Services.obs.removeObserver(this, httpTopic);
+    onShutdown.add(() =>
+    {
       Services.obs.removeObserver(this, "content-document-global-created");
 
       for (let category of this.xpcom_categories)
         catMan.deleteCategoryEntry(category, this.contractID, false);
 
       registrar.unregisterFactory(this.classID, this);
-
-      this.previousRequest = null;
-    }.bind(this));
+    });
   },
 
   //
   // nsISupports interface implementation
   //
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIContentPolicy, Ci.nsIObserver,
     Ci.nsIChannelEventSink, Ci.nsIFactory, Ci.nsISupportsWeakReference]),
 
   //
@@ skipping 17 matching lines @@
     // Ignore whitelisted schemes
     let location = Utils.unwrapURL(contentLocation);
     if (!Policy.isBlockableScheme(location))
       return Ci.nsIContentPolicy.ACCEPT;
 
     // Interpret unknown types as "other"
     if (!(contentType in Policy.typeDescr))
       contentType = Policy.type.OTHER;
 
     let result = Policy.processNode(wnd, node, contentType, location, false);
-    if (result)
-    {
-      // We didn't block this request so we will probably see it again in
-      // http-on-opening-request. Keep it so that we can associate it with the
-      // channel there - will be needed in case of redirect.
-      this.previousRequest = [location, contentType];
-    }
     return (result ? Ci.nsIContentPolicy.ACCEPT : Ci.nsIContentPolicy.REJECT_REQUEST);
   },
 
   shouldProcess: function(contentType, contentLocation, requestOrigin, insecNode, mimeType, extra)
   {
     return Ci.nsIContentPolicy.ACCEPT;
   },
 
   //
   // nsIObserver interface implementation
@@ skipping 10 matching lines @@
         let uri = additional || Utils.makeURI(subject.location.href);
         if (!Policy.processNode(subject.opener, subject.opener.document, Policy.type.POPUP, uri, false))
         {
           subject.stop();
           Utils.runAsync(() => subject.close());
         }
         else if (uri.spec == "about:blank")
         {
           // An about:blank pop-up most likely means that a load will be
           // initiated asynchronously. Wait for that.
-          Utils.runAsync(function()
+          Utils.runAsync(() =>
           {
             let channel = subject.QueryInterface(Ci.nsIInterfaceRequestor)
                                  .getInterface(Ci.nsIDocShell)
                                  .QueryInterface(Ci.nsIDocumentLoader)
                                  .documentChannel;
             if (channel)
               this.observe(subject, topic, data, channel.URI);
           });
         }
         break;
       }
-      case "http-on-opening-request":
-      case "http-on-modify-request":
-      {
-        if (!(subject instanceof Ci.nsIHttpChannel))
-          return;
-
-        if (this.previousRequest && subject.URI == this.previousRequest[0] &&
-            subject instanceof Ci.nsIWritablePropertyBag)
-        {
-          // We just handled a content policy call for this request - associate
-          // the data with the channel so that we can find it in case of a redirect.
-          subject.setProperty("abpRequestType", this.previousRequest[1]);
-          this.previousRequest = null;
-        }
-        break;
-      }
     }
   },
 
   //
   // nsIChannelEventSink interface implementation
   //
 
   asyncOnChannelRedirect: function(oldChannel, newChannel, flags, callback)
   {
     let result = Cr.NS_OK;
     try
     {
-      // Try to retrieve previously stored request data from the channel
-      let contentType;
-      if (oldChannel instanceof Ci.nsIWritablePropertyBag)
-      {
-        try
-        {
-          contentType = oldChannel.getProperty("abpRequestType");
-        }
-        catch(e)
-        {
-          // No data attached, ignore this redirect
-          return;
-        }
-      }
-
-      let newLocation = null;
-      try
-      {
-        newLocation = newChannel.URI;
-      } catch(e2) {}
-      if (!newLocation)
+      // nsILoadInfo.contentPolicyType was introduced in Gecko 35, then
+      // renamed to nsILoadInfo.externalContentPolicyType in Gecko 44.
+      let loadInfo = oldChannel.loadInfo;
+      let contentType = ("externalContentPolicyType" in loadInfo ?
+          loadInfo.externalContentPolicyType : loadInfo.contentPolicyType);
+      if (!contentType)
         return;
 
       let wnd = Utils.getRequestWindow(newChannel);
       if (!wnd)
         return;
 
       if (contentType == Ci.nsIContentPolicy.TYPE_DOCUMENT)

[Wladimir Palant, 2015/10/26 20:50:26]

This relies on the changes from https://codereview.adblockplus.org/29329409/,
before these changes the type here would be TYPE_POPUP rather than
TYPE_DOCUMENT.

       {
         if (wnd.history.length <= 1 && wnd.opener)
         {
-          // Special treatment for pop-up windows
-          this.observe(wnd, "content-document-global-created", null, newLocation);
+          // Special treatment for pop-up windows - this will close the window
+          // rather than preventing the redirect. Note that we might not have
+          // seen the original channel yet because the redirect happened before
+          // the async code in observe() had a chance to run.
+          this.observe(wnd, "content-document-global-created", null, oldChannel.URI);
+          this.observe(wnd, "content-document-global-created", null, newChannel.URI);
         }
         return;
       }
 
-      if (!Policy.processNode(wnd, wnd.document, contentType, newLocation, false))
+      if (!Policy.processNode(wnd, wnd.document, contentType, newChannel.URI, false))
         result = Cr.NS_BINDING_ABORTED;
     }
     catch (e)
     {
       // We shouldn't throw exceptions here - this will prevent the redirect.
       Cu.reportError(e);
     }
     finally
     {
       callback.onRedirectVerifyCallback(result);
@@ skipping 198 matching lines @@
   if (!wnd || wnd.closed)
     return;
 
   if (entry.type == Policy.type.OBJECT)
   {
     node.removeEventListener("mouseover", objectMouseEventHander, true);
     node.removeEventListener("mouseout", objectMouseEventHander, true);
   }
   Policy.processNode(wnd, node, entry.type, Utils.makeURI(entry.location), true);
 }