Bug 1338860 fix onErrorOccurred to handle some additional errors, r=aswan,kmag
authorShane Caraveo <scaraveo@mozilla.com>
Fri, 02 Jun 2017 14:16:48 -0700
changeset 410233 3c59f4cceb9a7f4acca9331409c91d148405382c
parent 410232 5092c00facfaacf3f761a5c9384b1b639d5de41d
child 410234 2c0e0f3062aeb2e1bca2b743ab0d82ca19619c9e
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan, kmag
bugs1338860
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1338860 fix onErrorOccurred to handle some additional errors, r=aswan,kmag MozReview-Commit-ID: I5uZmhWFBUd
toolkit/components/extensions/test/mochitest/chrome.ini
toolkit/components/extensions/test/mochitest/test_chrome_ext_webrequest_errors.html
toolkit/modules/addons/WebRequest.jsm
--- a/toolkit/components/extensions/test/mochitest/chrome.ini
+++ b/toolkit/components/extensions/test/mochitest/chrome.ini
@@ -25,16 +25,17 @@ skip-if = (toolkit == 'android') # andro
 skip-if = os == 'android' # unsupported.
 [test_chrome_ext_permissions.html]
 skip-if = os == 'android' # Bug 1350559
 [test_chrome_ext_storage_cleanup.html]
 [test_chrome_ext_trackingprotection.html]
 [test_chrome_ext_trustworthy_origin.html]
 [test_chrome_ext_webnavigation_resolved_urls.html]
 [test_chrome_ext_webrequest_background_events.html]
+[test_chrome_ext_webrequest_errors.html]
 [test_chrome_ext_webrequest_host_permissions.html]
 [test_chrome_native_messaging_paths.html]
 skip-if = os != "mac" && os != "linux"
 [test_ext_cookies_expiry.html]
 [test_ext_cookies_permissions_bad.html]
 [test_ext_cookies_permissions_good.html]
 [test_ext_cookies_containers.html]
 [test_ext_jsversion.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_webrequest_errors.html
@@ -0,0 +1,61 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for WebRequest errors</title>
+  <script type="text/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="chrome://mochikit/content/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="chrome_head.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<script type="text/javascript">
+"use strict";
+
+async function test_connection_refused(url, expectedError) {
+  async function background(url, expectedError) {
+    browser.test.log(`background url is ${url}`);
+    browser.webRequest.onErrorOccurred.addListener(details => {
+      if (details.url != url) {
+        return;
+      }
+      browser.test.assertTrue(details.error.startsWith(expectedError), "error correct");
+      browser.test.sendMessage("onErrorOccurred");
+    }, {urls: ["<all_urls>"]});
+
+    let tabId;
+    browser.test.onMessage.addListener(async (msg, expected) => {
+      await browser.tabs.remove(tabId);
+      browser.test.sendMessage("done");
+    });
+
+    let tab = await browser.tabs.create({url});
+    tabId = tab.id;
+  }
+
+  let extensionData = {
+    manifest: {
+      permissions: ["webRequest", "tabs", "*://badchain.include-subdomains.pinning.example.com/*"],
+    },
+    background: `(${background})("${url}", "${expectedError}")`,
+  };
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  await extension.startup();
+
+  await extension.awaitMessage("onErrorOccurred");
+  extension.sendMessage("close-tab");
+  await extension.awaitMessage("done");
+
+  await extension.unload();
+}
+
+add_task(function test_bad_cert() {
+  return test_connection_refused("https://badchain.include-subdomains.pinning.example.com/", "Unable to communicate securely with peer");
+});
+
+</script>
+
+</body>
+</html>
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -13,16 +13,20 @@ const Cc = Components.classes;
 const Cu = Components.utils;
 const Cr = Components.results;
 
 const {nsIHttpActivityObserver, nsISocketTransport} = Ci;
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
+XPCOMUtils.defineLazyServiceGetter(this, "NSSErrorsService",
+                                   "@mozilla.org/nss_errors_service;1",
+                                   "nsINSSErrorsService");
+
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionUtils",
                                   "resource://gre/modules/ExtensionUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "WebRequestCommon",
                                   "resource://gre/modules/WebRequestCommon.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "WebRequestUpload",
                                   "resource://gre/modules/WebRequestUpload.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "ExtensionError", () => ExtensionUtils.ExtensionError);
@@ -647,16 +651,24 @@ HttpObserverManager = {
     }
     delete this.activityErrorsMap;
     this.activityErrorsMap = map;
     return this.activityErrorsMap;
   },
   GOOD_LAST_ACTIVITY: nsIHttpActivityObserver.ACTIVITY_SUBTYPE_RESPONSE_HEADER,
   observeActivity(channel, activityType, activitySubtype /* , aTimestamp, aExtraSizeData, aExtraStringData */) {
     let channelData = getData(channel);
+
+    // StartStopListener has to be activated early in the request to catch
+    // SSL connection issues which do not get reported via nsIHttpActivityObserver.
+    if (activityType == nsIHttpActivityObserver.ACTIVITY_TYPE_HTTP_TRANSACTION &&
+        activitySubtype == nsIHttpActivityObserver.ACTIVITY_SUBTYPE_REQUEST_HEADER) {
+      this.attachStartStopListener(channel, channelData);
+    }
+
     let lastActivity = channelData.lastActivity || 0;
     if (activitySubtype === nsIHttpActivityObserver.ACTIVITY_SUBTYPE_RESPONSE_COMPLETE &&
         lastActivity && lastActivity !== this.GOOD_LAST_ACTIVITY) {
       let loadContext = this.getLoadContext(channel);
       if (!this.errorCheck(channel, loadContext, channelData)) {
         this.runChannelListener(channel, loadContext, "onError",
           {error: this.activityErrorsMap.get(lastActivity) ||
                   `NS_ERROR_NET_UNKNOWN_${lastActivity}`});
@@ -673,19 +685,26 @@ HttpObserverManager = {
   },
 
   get resultsMap() {
     delete this.resultsMap;
     this.resultsMap = new Map(Object.keys(Cr).map(name => [Cr[name], name]));
     return this.resultsMap;
   },
   maybeError(channel, extraData = null, channelData = null) {
+    if (!(extraData && extraData.error) && channel.securityInfo) {
+      let securityInfo = channel.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo);
+      if (NSSErrorsService.isNSSErrorCode(securityInfo.errorCode)) {
+        let nsresult = NSSErrorsService.getXPCOMFromNSSError(securityInfo.errorCode);
+        extraData = {error: NSSErrorsService.getErrorMessage(nsresult)};
+      }
+    }
     if (!(extraData && extraData.error)) {
       if (!Components.isSuccessCode(channel.status)) {
-        extraData = {error: this.resultsMap.get(channel.status)};
+        extraData = {error: this.resultsMap.get(channel.status) || "NS_ERROR_NET_UNKNOWN"};
       }
     }
     return extraData;
   },
   errorCheck(channel, loadContext, channelData = null) {
     let errorData = this.maybeError(channel, null, channelData);
     if (errorData) {
       this.runChannelListener(channel, loadContext, "onError", errorData);
@@ -987,35 +1006,42 @@ HttpObserverManager = {
     for (let opts of listener.values()) {
       if (this.shouldRunListener(policyType, uri, opts.filter)) {
         return true;
       }
     }
     return false;
   },
 
+  attachStartStopListener(channel, channelData) {
+    // Check whether we've already added a listener to this channel,
+    // so we don't wind up chaining multiple listeners.
+    if (!this.needTracing || channelData.hasListener || !(channel instanceof Ci.nsITraceableChannel)) {
+      return;
+    }
+    let responseStatus = 0;
+    try {
+      responseStatus = channel.QueryInterface(Ci.nsIHttpChannel).responseStatus;
+    } catch (e) {
+      /* NS_ERROR_NOT_AVAILABLE if checked prior to onStartRequest. */
+    }
+    // skip redirections, https://bugzilla.mozilla.org/show_bug.cgi?id=728901#c8
+    if (responseStatus < 300 || responseStatus >= 400) {
+      let loadContext = this.getLoadContext(channel);
+      new StartStopListener(this, channel, loadContext);
+      channelData.hasListener = true;
+    }
+  },
+
   examine(channel, topic, data) {
-    let loadContext = this.getLoadContext(channel);
-
     let channelData = getData(channel);
-    if (this.needTracing) {
-      // Check whether we've already added a listener to this channel,
-      // so we don't wind up chaining multiple listeners.
-      if (!channelData.hasListener && channel instanceof Ci.nsITraceableChannel) {
-        let responseStatus = channel.responseStatus;
-        // skip redirections, https://bugzilla.mozilla.org/show_bug.cgi?id=728901#c8
-        if (responseStatus < 300 || responseStatus >= 400) {
-          new StartStopListener(this, channel, loadContext);
-          channelData.hasListener = true;
-        }
-      }
-    }
+    this.attachStartStopListener(channel, channelData);
 
     if (this.listeners.headersReceived.size) {
-      this.runChannelListener(channel, loadContext, "headersReceived");
+      this.runChannelListener(channel, this.getLoadContext(channel), "headersReceived");
     }
 
     if (!channelData.hasAuthRequestor && this.shouldHookListener(this.listeners.authRequired, channel)) {
       channel.notificationCallbacks = new AuthRequestor(channel, this);
       channelData.hasAuthRequestor = true;
     }
   },