commit d7cb80eac503791f9026b7ce21bcaedd72d6181c Author: Georg Koppen <gk@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; });