[Pkg-mozext-commits] [requestpolicy] 74/280: add `HttpResponse`, refactoring and fix #351

David Prévot taffit at moszumanska.debian.org
Sat May 2 20:30:01 UTC 2015


This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to branch master
in repository requestpolicy.

commit 69b12432fee1cf6c4ea9c29a5c4a7a71f83f6e6d
Author: Martin Kimmerle <dev at 256k.de>
Date:   Tue Dec 30 23:22:50 2014 +0100

    add `HttpResponse`, refactoring and fix #351
    
    The `HttpResponse` class has been added.
    Some code of `request-processor.redirects.js` has been moved
    into that class.
---
 src/content/lib/http-response.jsm                  | 162 ++++++++++++++++++
 src/content/lib/request-processor.redirects.js     | 187 +++++++--------------
 src/content/lib/request.jsm                        |  15 +-
 src/content/ui/overlay.js                          |  14 --
 .../mozmill/tests/testRedirect/testAutoRedirect.js |  14 +-
 5 files changed, 244 insertions(+), 148 deletions(-)

diff --git a/src/content/lib/http-response.jsm b/src/content/lib/http-response.jsm
new file mode 100644
index 0000000..a8b5fdc
--- /dev/null
+++ b/src/content/lib/http-response.jsm
@@ -0,0 +1,162 @@
+/*
+ * ***** BEGIN LICENSE BLOCK *****
+ *
+ * RequestPolicy - A Firefox extension for control over cross-site requests.
+ * Copyright (c) 2008-2012 Justin Samuel
+ * Copyright (c) 2014 Martin Kimmerle
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation, either version 3 of the License, or (at your option) any later
+ * version.
+ *
+ * This program 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
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * ***** END LICENSE BLOCK *****
+ */
+
+const Ci = Components.interfaces;
+const Cc = Components.classes;
+const Cu = Components.utils;
+
+let EXPORTED_SYMBOLS = [
+  "HttpResponse"
+];
+
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+Cu.import("chrome://requestpolicy/content/lib/script-loader.jsm");
+ScriptLoader.importModules([
+  "logger",
+  "domain-util",
+  "utils"
+], this);
+
+
+
+function HttpResponse(aHttpChannel) {
+  this.httpChannel = aHttpChannel;
+
+  this.containsRedirection = undefined;
+  this.redirHeaderType = undefined;
+  this.redirHeaderValue = undefined;
+
+  // initialize
+  determineHttpHeader.call(this);
+  XPCOMUtils.defineLazyGetter(this, "rawDestString", getRawDestString);
+  XPCOMUtils.defineLazyGetter(this, "destURI", getDestURI);
+  XPCOMUtils.defineLazyGetter(this, "originURI", getOriginURI);
+  XPCOMUtils.defineLazyGetter(this, "browser", getBrowser);
+}
+
+HttpResponse.headerTypes = ["Location", "Refresh"];
+
+HttpResponse.prototype.removeResponseHeader = function() {
+  this.httpChannel.setResponseHeader(this.redirHeaderType, "", false);
+};
+
+
+
+
+
+/**
+ * This function calls getResponseHeader(headerType). If there is no such
+ * header, that function will throw NS_ERROR_NOT_AVAILABLE.
+ */
+function determineHttpHeader() {
+  this.containsRedirection = true;
+
+  for (let i in HttpResponse.headerTypes) {
+    try {
+      this.redirHeaderType = HttpResponse.headerTypes[i];
+      this.redirHeaderValue = this.httpChannel.getResponseHeader(this.redirHeaderType);
+      // In case getResponseHeader() didn't throw NS_ERROR_NOT_AVAILABLE,
+      // the header-type exists, so we return:
+      return;
+    } catch (e) {
+    }
+  }
+
+  // The following will be executed when there is no redirection:
+  this.containsRedirection = false;
+  this.redirHeaderType = null;
+  this.redirHeaderValue = null;
+}
+
+function getRawDestString() {
+  switch (this.redirHeaderType) {
+    case "Location":
+      return this.redirHeaderValue;
+
+    case "Refresh":
+      try {
+        // We can ignore the delay because we aren't manually doing
+        // the refreshes. Allowed refreshes we still leave to the browser.
+        // The rawDestString may be empty if the origin is what should be refreshed.
+        // This will be handled by DomainUtil.determineRedirectUri().
+        return DomainUtil.parseRefresh(this.redirHeaderValue).destURI;
+      } catch (e) {
+        Logger.warning(Logger.TYPE_HEADER_REDIRECT,
+            "Invalid refresh header: <" + this.redirHeaderValue + ">");
+        if (!Prefs.isBlockingDisabled()) {
+          this.removeResponseHeader();
+        }
+        return null;
+      }
+
+    default:
+      return null;
+  }
+}
+
+function getDestURI() {
+  return Services.io.newURI(this.rawDestString, null, this.originURI);
+}
+
+function getOriginURI() {
+  return Services.io.newURI(this.httpChannel.name, null, null);
+}
+
+/**
+ * Get the <browser> (nsIDOMXULElement) related to this request.
+ */
+function getBrowser() {
+  /* start - be careful when editing here */
+  let loadContext = null;
+  try {
+    loadContext = this.httpChannel.notificationCallbacks
+                                  .QueryInterface(Ci.nsIInterfaceRequestor)
+                                  .getInterface(Ci.nsILoadContext);
+  } catch (ex) {
+    try {
+      loadContext = this.httpChannel.loadGroup.notificationCallbacks
+          .getInterface(Ci.nsILoadContext);
+    } catch (ex2) {
+      return null;
+    }
+  }
+  /* end - be careful when editing here */
+
+
+  try {
+    if (loadContext.topFrameElement) {
+      // the top frame element should be already the browser element
+      return loadContext.topFrameElement;
+    } else {
+      // we hope the associated window is available. in multiprocessor
+      // firefox it's not available.
+      return Utils.getBrowserForWindow(loadContext.topWindow);
+    }
+  } catch (e) {
+    Logger.warning(Logger.TYPE_HEADER_REDIRECT, "The redirection's " +
+                   "Load Context couldn't be found! " + e);
+    return null;
+  }
+};
diff --git a/src/content/lib/request-processor.redirects.js b/src/content/lib/request-processor.redirects.js
index 1db3922..1052dbf 100644
--- a/src/content/lib/request-processor.redirects.js
+++ b/src/content/lib/request-processor.redirects.js
@@ -23,6 +23,7 @@
 
 const HTTPS_EVERYWHERE_REWRITE_TOPIC = "https-everywhere-uri-rewrite";
 
+Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 Cu.import("chrome://requestpolicy/content/lib/script-loader.jsm");
@@ -33,7 +34,8 @@ ScriptLoader.importModules([
   "domain-util",
   "utils",
   "request",
-  "request-result"
+  "request-result",
+  "http-response"
 ], this);
 ScriptLoader.defineLazyModuleGetters({
   "requestpolicy-service": ["rpService"]
@@ -169,17 +171,21 @@ let RequestProcessor = (function(self) {
 
 
   self.isAllowedRedirect = function(originURI, destURI) {
-    var request = new RedirectRequest(originURI, destURI);
+    var request = new Request(originURI, destURI);
     return (true === checkRedirect(request).isAllowed);
   };
 
-  function processRedirect(request, httpChannel) {
+
+
+  function processUrlRedirection(request) {
+    let httpResponse = request.httpResponse;
+    let httpChannel = httpResponse.httpChannel;
     var originURI = request.originURI;
     var destURI = request.destURI;
-    var headerType = request.httpHeader;
+    var headerType = httpResponse.redirHeaderType;
 
     // Ignore redirects to javascript. The browser will ignore them, as well.
-    if (DomainUtil.getUriObject(destURI).schemeIs("javascript")) {
+    if (httpResponse.destURI.schemeIs("javascript")) {
       Logger.warning(Logger.TYPE_HEADER_REDIRECT,
           "Ignoring redirect to javascript URI <" + destURI + ">");
       return;
@@ -226,61 +232,28 @@ let RequestProcessor = (function(self) {
     // The header isn't allowed, so remove it.
     try {
       if (!Prefs.isBlockingDisabled()) {
-        httpChannel.setResponseHeader(headerType, "", false);
-
-        /* start - do not edit here */
-        let interfaceRequestor = httpChannel.notificationCallbacks
-            .QueryInterface(Ci.nsIInterfaceRequestor);
-        let loadContext = null;
-        try {
-          loadContext = interfaceRequestor.getInterface(Ci.nsILoadContext);
-        } catch (ex) {
-          try {
-            loadContext = aSubject.loadGroup.notificationCallbacks
-                .getInterface(Ci.nsILoadContext);
-          } catch (ex2) {}
-        }
-        /*end do not edit here*/
-
-
-        let browser;
-        try {
-          if (loadContext.topFrameElement) {
-            // the top frame element should be already the browser element
-            browser = loadContext.topFrameElement;
-          } else {
-            // we hope the associated window is available. in multiprocessor
-            // firefox it's not available.
-            browser = Utils.getBrowserForWindow(loadContext.topWindow);
-          }
-          // save all blocked redirects directly in the browser element. the
-          // blocked elements will be checked later when the DOM content
-          // finished loading.
-          browser.requestpolicy = browser.requestpolicy || {blockedRedirects: {}};
-          browser.requestpolicy.blockedRedirects[originURI] = destURI;
-        } catch (e) {
-          Logger.warning(Logger.TYPE_HEADER_REDIRECT, "The redirection's " +
-                         "Load Context couldn't be found! " + e);
-        }
+        httpResponse.removeResponseHeader();
 
+        let browser = request.browser;
 
-        try {
-          let contentDisp = httpChannel.getResponseHeader("Content-Disposition");
-          if (contentDisp.indexOf("attachment") != -1) {
-            try {
-              httpChannel.setResponseHeader("Content-Disposition", "", false);
-              Logger.warning(Logger.TYPE_HEADER_REDIRECT,
-                  "Removed 'Content-Disposition: attachment' header to " +
-                  "prevent display of about:neterror.");
-            } catch (e) {
-              Logger.warning(Logger.TYPE_HEADER_REDIRECT,
-                  "Unable to remove 'Content-Disposition: attachment' header " +
-                  "to prevent display of about:neterror. " + e);
-            }
-          }
-        } catch (e) {
-          // No Content-Disposition header.
-        }
+        // TODO: use WeakMap instead of putting data into the <browser> object
+        // see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakMap
+
+        // save all blocked redirects directly in the browser element. the
+        // blocked elements will be checked later when the DOM content
+        // finished loading.
+        browser.requestpolicy = browser.requestpolicy || {blockedRedirects: {}};
+        browser.requestpolicy.blockedRedirects[originURI] = destURI;
+
+        // Cancel the request. As of Fx 37, this causes the location bar to
+        // show the URL of the previously displayed page.
+        httpChannel.cancel(Cr.NS_BINDING_ABORTED);
+
+        showRedirectNotification(request) || Logger.warning(
+            Logger.TYPE_HEADER_REDIRECT,
+            "A redirect has been observed, but it was not possible to notify " +
+            "the user! The redirect was from page <" + request.originURI + "> " +
+            "to <" + request.destURI + ">.");
 
         // We try to trace the blocked redirect back to a link click or form
         // submission if we can. It may indicate, for example, a link that
@@ -306,9 +279,6 @@ let RequestProcessor = (function(self) {
             var sourcePage = i;
           }
 
-          notifyRequestObserversOfBlockedLinkClickRedirect(sourcePage,
-              originURI, destURI);
-
           // Maybe we just record the clicked link and each step in between as
           // an allowed request, and the final blocked one as a blocked request.
           // That is, make it show up in the requestpolicy menu like anything
@@ -336,19 +306,23 @@ let RequestProcessor = (function(self) {
     }
   }
 
-  function notifyRequestObserversOfBlockedLinkClickRedirect(sourcePageUri,
-      linkDestUri, blockedRedirectUri) {
-    for (var i = 0; i < internal.requestObservers.length; i++) {
-      if (!internal.requestObservers[i]) {
-        continue;
-      }
-      internal.requestObservers[i].observeBlockedLinkClickRedirect(
-          sourcePageUri, linkDestUri, blockedRedirectUri);
+  function showRedirectNotification(request) {
+    let browser = request.browser;
+    if (browser === null) {
+      return false;
     }
+
+    let window = browser.ownerGlobal;
+    let overlay = window.requestpolicy.overlay;
+    return overlay._showRedirectNotification(browser, request.destURI, 0,
+                                             request.originURI);
+    return true;
   }
 
 
 
+
+
   /**
    * Called after a response has been received from the web server. Headers are
    * available on the channel. The response can be accessed and modified via
@@ -366,88 +340,45 @@ let RequestProcessor = (function(self) {
     // TODO: Make user aware of blocked headers so they can allow them if
     // desired.
 
-    var httpChannel = aSubject.QueryInterface(Ci.nsIHttpChannel);
+    let httpChannel = aSubject.QueryInterface(Ci.nsIHttpChannel);
+    let httpResponse = new HttpResponse(httpChannel);
 
-    var headerType;
-    var dest;
+    // the "raw" dest string might be a relative or absolute URI
+    let rawDestString = httpResponse.rawDestString;
 
-    try {
-      // If there is no such header, getResponseHeader() will throw
-      // NS_ERROR_NOT_AVAILABLE. If there is more than header, the last one is
-      // the one that will be used.
-      headerType = "Location";
-      dest = httpChannel.getResponseHeader(headerType);
-    } catch (e) {
-      // No location header. Look for a Refresh header.
-      try {
-        headerType = "Refresh";
-        var refreshString = httpChannel.getResponseHeader(headerType);
-      } catch (e) {
-        // No Location header or Refresh header.
-        return;
-      }
-      try {
-        // We can ignore the delay because we aren't manually doing
-        // the refreshes. Allowed refreshes we still leave to the browser.
-        // The dest may be empty if the origin is what should be refreshed.
-        // This will be handled by DomainUtil.determineRedirectUri().
-        var dest = DomainUtil.parseRefresh(refreshString).destURI;
-      } catch (e) {
-        Logger.warning(Logger.TYPE_HEADER_REDIRECT,
-            "Invalid refresh header: <" + refreshString + ">");
-        if (!Prefs.isBlockingDisabled()) {
-          httpChannel.setResponseHeader(headerType, "", false);
-        }
-        return;
-      }
+    if (httpResponse.containsRedirection === false || rawDestString === null) {
+      return;
     }
 
-    // For origins that are IDNs, this will always be in ACE format. We want
-    // it in UTF8 format if it's a TLD that Mozilla allows to be in UTF8.
-    var originURI = DomainUtil.formatIDNUri(httpChannel.name);
+    let originString = httpResponse.originURI.specIgnoringRef;
 
     // Allow redirects of requests from privileged code.
     if (!isContentRequest(httpChannel)) {
       // However, favicon requests that are redirected appear as non-content
       // requests. So, check if the original request was for a favicon.
-      var originPath = DomainUtil.getPath(httpChannel.name);
+      var originPath = httpResponse.originURI.path;
       // We always have to check "/favicon.ico" because Firefox will use this
       // as a default path and that request won't pass through shouldLoad().
-      if (originPath == "/favicon.ico" || internal.faviconRequests[originURI]) {
+      if (originPath == "/favicon.ico" || internal.faviconRequests[originString]) {
         // If the redirected request is allowed, we need to know that was a
         // favicon request in case it is further redirected.
-        internal.faviconRequests[dest] = true;
+        internal.faviconRequests[rawDestString] = true;
         Logger.info(Logger.TYPE_HEADER_REDIRECT, "'" + headerType
-                + "' header to <" + dest + "> " + "from <" + originURI
+                + "' header to <" + rawDestString + "> " + "from <" + originString
                 + "> appears to be a redirected favicon request. "
                 + "This will be treated as a content request.");
       } else {
         Logger.warning(Logger.TYPE_HEADER_REDIRECT,
-            "** ALLOWED ** '" + headerType + "' header to <" + dest + "> " +
-            "from <" + originURI +
+            "** ALLOWED ** '" + httpResponse.redirHeaderType +
+            "' header to <" + rawDestString + "> " +
+            "from <" + originString +
             ">. Original request is from privileged code.");
         return;
       }
     }
 
-    // If it's not a valid uri, the redirect is relative to the origin host.
-    // The way we have things written currently, without this check the full
-    // dest string will get treated as the destination and displayed in the
-    // menu because DomainUtil.getIdentifier() doesn't raise exceptions.
-    // We add this to fix issue #39:
-    // https://github.com/RequestPolicyContinued/requestpolicy/issues/39
-    if (!DomainUtil.isValidUri(dest)) {
-      var destAsUri = DomainUtil.determineRedirectUri(originURI, dest);
-      Logger.warning(
-          Logger.TYPE_HEADER_REDIRECT,
-          "Redirect destination is not a valid uri, assuming dest <" + dest
-              + "> from origin <" + originURI + "> is actually dest <" + destAsUri
-              + ">.");
-      dest = destAsUri;
-    }
-
-    var request = new RedirectRequest(originURI, dest, headerType);
-    processRedirect(request, httpChannel);
+    var request = new RedirectRequest(httpResponse);
+    processUrlRedirection(request);
   };
 
 
diff --git a/src/content/lib/request.jsm b/src/content/lib/request.jsm
index 6efec6b..1378e02 100644
--- a/src/content/lib/request.jsm
+++ b/src/content/lib/request.jsm
@@ -33,6 +33,8 @@ let EXPORTED_SYMBOLS = [
   "REQUEST_TYPE_REDIRECT"
 ];
 
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
 Cu.import("chrome://requestpolicy/content/lib/script-loader.jsm");
 ScriptLoader.importModules([
   "logger",
@@ -47,6 +49,7 @@ const REQUEST_TYPE_REDIRECT = 2;
 
 
 function Request(originURI, destURI, requestType) {
+  // TODO: save a nsIURI objects here instead of strings
   this.originURI = originURI;
   this.destURI = destURI;
   this.requestType = requestType;
@@ -348,10 +351,16 @@ NormalRequest.prototype.checkURISchemes = function() {
 
 
 
-function RedirectRequest(originURI, destURI, httpHeader) {
-  Request.call(this, originURI, destURI, REQUEST_TYPE_REDIRECT);
+function RedirectRequest(httpResponse) {
+  Request.call(this, httpResponse.originURI.specIgnoringRef,
+               httpResponse.destURI.specIgnoringRef, REQUEST_TYPE_REDIRECT);
+  this.httpResponse = httpResponse;
 
-  this.httpHeader = httpHeader;
+  XPCOMUtils.defineLazyGetter(this, "browser", getBrowser);
 }
 RedirectRequest.prototype = Object.create(Request.prototype);
 RedirectRequest.prototype.constructor = Request;
+
+function getBrowser() {
+  return this.httpResponse.browser;
+}
diff --git a/src/content/ui/overlay.js b/src/content/ui/overlay.js
index bd3682e..7203ee9 100644
--- a/src/content/ui/overlay.js
+++ b/src/content/ui/overlay.js
@@ -660,20 +660,6 @@ requestpolicy.overlay = (function() {
     }
   };
 
-  self.observeBlockedLinkClickRedirect = function(sourcePageUri, linkDestUri,
-      blockedRedirectUri) {
-    // TODO: Figure out a good way to notify the user. For now, it should at
-    // least be showing up in the menu the first time it happens. After that,
-    // some caching issues seem to get in the way and the blocked request
-    // isn't tried again, so there's no awareness of it.
-    Logger.warning(Logger.TYPE_HEADER_REDIRECT,
-        "Observed blocked link click redirect from page <" + sourcePageUri
-            + "> with redirect origin <" + linkDestUri
-            + "> and redirect dest <" + blockedRedirectUri
-            + ">. --- WARNING: other than the menu "
-            + "showing this blocked request, there is no other indication.");
-  };
-
   /**
    * If the RP service noticed a blocked top-level document request, look for
    * a tab where the current location is the same as the origin of the blocked
diff --git a/tests/mozmill/tests/testRedirect/testAutoRedirect.js b/tests/mozmill/tests/testRedirect/testAutoRedirect.js
index 7a082df..ce873c3 100644
--- a/tests/mozmill/tests/testRedirect/testAutoRedirect.js
+++ b/tests/mozmill/tests/testRedirect/testAutoRedirect.js
@@ -8,6 +8,7 @@ var {assert, expect} = require("../../../../../../lib/assertions");
 var prefs = require("../../../../../lib/prefs");
 var tabs = require("../../../../../lib/tabs");
 
+var rpUtils = require("../../lib/rp-utils");
 var rpConst = require("../../lib/constants");
 
 var testURLPrePath = "http://www.maindomain.test/";
@@ -50,12 +51,17 @@ var testAutoRedirect = function() {
       '/{"value":"' + rpConst.REDIRECT_NOTIFICATION_VALUE + '"}');
 
   for (let [shouldBeAllowed, testURL] of urlsWithRedirect) {
+    let initialURI = controller.window.content.document.location.href;
     testURL = testURLPrePath + testURL;
     dump("Testing " + testURL + ". The redirect should be " +
          (shouldBeAllowed ? "allowed" : "blocked") + ".\n");
 
     controller.open(testURL);
-    controller.waitForPageLoad();
+
+    // Wait for the tab to be loaded. The function `waitForPageLoad()` will not
+    // work here, because if a redirect is blocked by cancelling the channel's
+    // associated request, the `waitForPageLoad` would wait infinitely.
+    rpUtils.waitForTabLoad(controller, tabBrowser.getTab(tabIndex));
 
     if (shouldBeAllowed) {
       controller.waitFor(function() {
@@ -63,8 +69,10 @@ var testAutoRedirect = function() {
       }, "The URL in the urlbar has changed.");
       expect.ok(!panel.exists(), "The redirect notification bar is hidden.");
     } else {
-      expect.ok(panel.exists(), "The redirect notification bar is displayed.");
-      expect.ok(controller.window.content.document.location.href === testURL,
+      controller.waitFor((() => panel.exists()), "The redirect " +
+                           "notification bar has been displayed.");
+      let finalURI = controller.window.content.document.location.href;
+      expect.ok(finalURI === testURL || finalURI === initialURI,
                 "The URL in the urlbar hasn't changed.");
     }
 

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-mozext/requestpolicy.git



More information about the Pkg-mozext-commits mailing list