[tor-commits] [torbutton/master] Bug 9901: Fix freeze due to content type sniffing

mikeperry at torproject.org mikeperry at torproject.org
Thu Feb 27 23:04:23 UTC 2014


commit d7cb80eac503791f9026b7ce21bcaedd72d6181c
Author: Georg Koppen <gk at torproject.org>
Date:   Wed Feb 12 12:54:08 2014 +0100

    Bug 9901: Fix freeze due to content type sniffing
    
    The external application blocker code suppresses error messages that are
    triggered by the JS -> C++ bridge. The downside is that content type
    sniffing leads to browser freezes. This patch overrides Firefox methods
    in order to gain the former while avoiding the latter.
---
 src/chrome/content/torbutton.js        |  100 ++++++++++++++++++++++++++++++++
 src/components/external-app-blocker.js |   24 ++------
 2 files changed, 106 insertions(+), 18 deletions(-)

diff --git a/src/chrome/content/torbutton.js b/src/chrome/content/torbutton.js
index 1f8e82c..f2fcb77 100644
--- a/src/chrome/content/torbutton.js
+++ b/src/chrome/content/torbutton.js
@@ -6,6 +6,13 @@
 // TODO: Double-check there are no strange exploits to defeat:
 //       http://kb.mozillazine.org/Links_to_local_pages_don%27t_work
 
+XPCOMUtils.defineLazyModuleGetter(this, "HUDService",
+  "resource:///modules/HUDService.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "ConsoleServiceListener",
+  "resource://gre/modules/devtools/WebConsoleUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "WebConsoleUtils",
+  "resource://gre/modules/devtools/WebConsoleUtils.jsm");
+
 const k_tb_browser_update_needed_pref = "extensions.torbutton.updateNeeded";
 const k_tb_tor_check_failed_topic = "Torbutton:TorCheckFailed";
 
@@ -2514,12 +2521,104 @@ function torbutton_is_windowed(wind) {
     return true;
 }
 
+// This is the console observer used for getting unwanted error messages
+// resulting from JS -> C++ transition filtered out.
+var consoleObserver = {
+
+  obs : null,
+
+  register: function() {
+    this.obs = Cc["@mozilla.org/observer-service;1"].
+      getService(Ci.nsIObserverService);
+    this.obs.addObserver(this, "web-console-created", false);
+  },
+
+  unregister: function() {
+    if (this.obs) {
+      this.obs.removeObserver(this, "web-console-created");
+    }
+  },
+
+  observe: function(subject, topic, data) {
+    if (topic === "web-console-created") {
+      var id = subject.QueryInterface(Ci.nsISupportsString).toString();
+      var con = HUDService.getHudReferenceById(subject);
+      con.ui.reportPageErrorOld = con.ui.reportPageError;
+      // Filtering the messages by making them hidden adding the
+      // "hidden-message" class. If the message does not need to get filtered
+      // the original method is executed without any modifications.
+      con.ui.reportPageError =
+        function WCF_reportPageError(aCategory, aScriptError) {
+          var message = aScriptError.errorMessage;
+          if (message && message.indexOf("NS_ERROR_NOT_AVAILABLE") > -1 &&
+              message.indexOf("external-app-blocker.js") > -1) {
+            return this.reportPageErrorOld(aCategory, aScriptError).classList.
+              add("hidden-message");
+          } else {
+            return this.reportPageErrorOld(aCategory, aScriptError);
+          }
+        }
+    }
+  }
+};
+
+// Ideally, we only need to patch/override one method to avoid errors showing up
+// in the browser console. Alas, that is not as easy given the presence of
+// cached messages and the Web Console which we need to consider as well while
+// overriding Devtool methods. Thus, we patch the code path that is called when
+// the browser console is already open AND additionally the one when cached
+// messages are displayed.
+function handleConsole() {
+  consoleObserver.register();
+  try {
+    // Filtering using the "web-console-created" notification is not enough as
+    // the cached messages are already loaded when it is fired. Therefore,
+    // change |getCachedMessages()| slighty to fit the needs at hand.
+    // The original code is https://mxr.mozilla.org/mozilla-esr24/source/
+    // toolkit/devtools/webconsole/WebConsoleUtils.jsm#998 ff. and distributed
+    // under the MPL 2.0 license.
+    ConsoleServiceListener.prototype.getCachedMessages =
+      function CSL_getCachedMessages(aIncludePrivate = false) {
+        var innerWindowID = this.window ? WebConsoleUtils.
+          getInnerWindowId(this.window) : null;
+        var errors = Services.console.getMessageArray() || [];
+
+        return errors.filter((aError) => {
+          if (aError instanceof Ci.nsIScriptError) {
+            var message = aError.message;
+            if (message && message.indexOf("NS_ERROR_NOT_AVAILABLE") > -1 &&
+                message.indexOf("external-app-blocker.js") > -1) {
+              return false;
+            }
+            if (!aIncludePrivate && aError.isFromPrivateWindow) {
+              return false;
+            }
+            if (innerWindowID &&
+                (aError.innerWindowID != innerWindowID ||
+                 !this.isCategoryAllowed(aError.category))) {
+              return false;
+            }
+          }
+          else if (innerWindowID) {
+            // If this is not an nsIScriptError and we need to do window-based
+            // filtering we skip this message.
+            return false;
+          }
+
+          return true;
+        });
+      };
+  } catch (e) {}
+}
+
 // Bug 1506 P3: This is needed pretty much only for the version check
 // and the window resizing. See comments for individual functions for
 // details
 function torbutton_new_window(event)
 {
     torbutton_log(3, "New window");
+    // Working around #9901, sigh...
+    handleConsole();
     var browser = getBrowser();
 
     if(!browser) {
@@ -2558,6 +2657,7 @@ function torbutton_new_window(event)
 function torbutton_close_window(event) {
     torbutton_window_pref_observer.unregister();
     torbutton_tor_check_observer.unregister();
+    consoleObserver.unregister();
 
     // TODO: This is a real ghetto hack.. When the original window
     // closes, we need to find another window to handle observing 
diff --git a/src/components/external-app-blocker.js b/src/components/external-app-blocker.js
index 1996179..057f4ae 100644
--- a/src/components/external-app-blocker.js
+++ b/src/components/external-app-blocker.js
@@ -122,24 +122,12 @@ ExternalWrapper.prototype =
           var call;
           if(params.length) call = "("+params.join().replace(/(?:)/g,function(){return "p"+(++x)})+")";
           else call = "()";
-          if(method == "getTypeFromFile" || method == "getTypeFromExtension" || method == "getTypeFromURI") {
-           // XXX: Due to https://developer.mozilla.org/en/Exception_logging_in_JavaScript
-           // this is necessary to prevent error console noise on the return to C++ code.
-           // It is not technically correct, but as far as I can tell, returning null
-           // here should be equivalent to throwing an error for the codepaths invovled
-           var fun = "(function "+call+"{"+
-              "if (arguments.length < "+wrapped[method].length+")"+
-              "  throw Components.results.NS_ERROR_XPC_NOT_ENOUGH_ARGS;"+
-              "try { return wrapped."+method+".apply(wrapped, arguments); }"+
-              "catch(e) { if(e.result == Components.results.NS_ERROR_NOT_AVAILABLE) return null; else throw e;} })";
-            newObj[method] = eval(fun);
-          } else {
-            var fun = "(function "+call+"{"+
-              "if (arguments.length < "+wrapped[method].length+")"+
-              "  throw Components.results.NS_ERROR_XPC_NOT_ENOUGH_ARGS;"+
-              "return wrapped."+method+".apply(wrapped, arguments);})";
-            newObj[method] = eval(fun);
-          }
+
+          var fun = "(function "+call+"{"+
+            "if (arguments.length < "+wrapped[method].length+")"+
+            "  throw Components.results.NS_ERROR_XPC_NOT_ENOUGH_ARGS;"+
+            "return wrapped."+method+".apply(wrapped, arguments);})";
+          newObj[method] = eval(fun);
        } else {
           newObj.__defineGetter__(method, function() { return wrapped[method]; });
           newObj.__defineSetter__(method, function(val) { wrapped[method] = val; });





More information about the tor-commits mailing list