Bug 1402944: Part 2 - Move error string logic into ChannelWrapper. r?mixedpuppy,ehsan draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 22 Sep 2017 18:43:18 -0700
changeset 670801 b7b80c928d8c9ce783cd0bcd3c36d98f0ca9aa26
parent 670800 d20236f9b72b40166a0861bf9ed364eb4ad83ef7
child 670802 3ab72773f74a7ef2ed6f9351d1490f89b023f21d
push id81714
push usermaglione.k@gmail.com
push dateTue, 26 Sep 2017 21:30:45 +0000
reviewersmixedpuppy, ehsan
bugs1402944
milestone58.0a1
Bug 1402944: Part 2 - Move error string logic into ChannelWrapper. r?mixedpuppy,ehsan MozReview-Commit-ID: 4rOeoliLTV7
dom/webidl/ChannelWrapper.webidl
netwerk/socket/nsITransportSecurityInfo.idl
security/manager/ssl/TransportSecurityInfo.cpp
security/manager/ssl/TransportSecurityInfo.h
toolkit/components/extensions/webrequest/ChannelWrapper.cpp
toolkit/components/extensions/webrequest/ChannelWrapper.h
toolkit/modules/addons/WebRequest.jsm
--- a/dom/webidl/ChannelWrapper.webidl
+++ b/dom/webidl/ChannelWrapper.webidl
@@ -136,16 +136,32 @@ interface ChannelWrapper {
    * will be an empty string if a response has not yet been received, or if
    * the request is not an HTTP request.
    */
   [Cached, Pure]
   readonly attribute ByteString statusLine;
 
 
   /**
+   * If the request has failed or been canceled, an opaque string representing
+   * the error. For requests that failed at the NSS layer, this is an NSS
+   * error message. For requests that failed for any other reason, it is the
+   * name of an nsresult error code. For requests which haven't failed, this
+   * is null.
+   *
+   * This string is used in the error message when notifying extension
+   * webRequest listeners of failure. The documentation specifically states
+   * that this value MUST NOT be parsed, and is only meant to be displayed to
+   * humans, but we all know how that works in real life.
+   */
+  [Cached, Pure]
+  readonly attribute DOMString? errorString;
+
+
+  /**
    * Information about the proxy server which is handling this request, or
    * null if the request is not proxied.
    */
   [Cached, Frozen, GetterThrows, Pure]
   readonly attribute MozProxyInfo? proxyInfo;
 
   /**
    * For HTTP requests, the IP address of the remote server handling the
--- a/netwerk/socket/nsITransportSecurityInfo.idl
+++ b/netwerk/socket/nsITransportSecurityInfo.idl
@@ -3,21 +3,21 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 
 interface nsIX509CertList;
 
-[scriptable, uuid(216112d3-28bc-4671-b057-f98cc09ba1ea)]
+[builtinclass, scriptable, uuid(216112d3-28bc-4671-b057-f98cc09ba1ea)]
 interface nsITransportSecurityInfo : nsISupports {
-    readonly attribute unsigned long    securityState;
-    readonly attribute wstring          errorMessage;
-    readonly attribute long             errorCode; // PRErrorCode
+    readonly attribute unsigned long securityState;
+    readonly attribute wstring errorMessage;
+    [infallible] readonly attribute long errorCode; // PRErrorCode
 
     /**
      * If certificate verification failed, this will be the peer certificate
      * chain provided in the handshake, so it can be used for error reporting.
      * If verification succeeded, this will be null.
      */
     readonly attribute nsIX509CertList failedCertChain;
 };
--- a/security/manager/ssl/TransportSecurityInfo.cpp
+++ b/security/manager/ssl/TransportSecurityInfo.cpp
@@ -84,22 +84,23 @@ TransportSecurityInfo::SetPort(int32_t a
 
 void
 TransportSecurityInfo::SetOriginAttributes(
   const OriginAttributes& aOriginAttributes)
 {
   mOriginAttributes = aOriginAttributes;
 }
 
-PRErrorCode
-TransportSecurityInfo::GetErrorCode() const
+NS_IMETHODIMP
+TransportSecurityInfo::GetErrorCode(int32_t* state)
 {
   MutexAutoLock lock(mMutex);
 
-  return mErrorCode;
+  *state = mErrorCode;
+  return NS_OK;
 }
 
 void
 TransportSecurityInfo::SetCanceled(PRErrorCode errorCode,
                                    SSLErrorMessageType errorMessageType)
 {
   MutexAutoLock lock(mMutex);
 
@@ -252,23 +253,16 @@ TransportSecurityInfo::formatErrorMessag
   if (NS_FAILED(rv)) {
     result.Truncate();
   }
 
   return rv;
 }
 
 NS_IMETHODIMP
-TransportSecurityInfo::GetErrorCode(int32_t* state)
-{
-  *state = GetErrorCode();
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 TransportSecurityInfo::GetInterface(const nsIID & uuid, void * *result)
 {
   if (!NS_IsMainThread()) {
     NS_ERROR("nsNSSSocketInfo::GetInterface called off the main thread");
     return NS_ERROR_NOT_SAME_THREAD;
   }
 
   nsresult rv;
--- a/security/manager/ssl/TransportSecurityInfo.h
+++ b/security/manager/ssl/TransportSecurityInfo.h
@@ -61,18 +61,16 @@ public:
   int32_t GetPort() const { return mPort; }
   void SetPort(int32_t aPort);
 
   const OriginAttributes& GetOriginAttributes() const {
     return mOriginAttributes;
   }
   void SetOriginAttributes(const OriginAttributes& aOriginAttributes);
 
-  PRErrorCode GetErrorCode() const;
-
   void GetErrorLogMessage(PRErrorCode errorCode,
                           ::mozilla::psm::SSLErrorMessageType errorMessageType,
                           nsString &result);
 
   void SetCanceled(PRErrorCode errorCode,
                    ::mozilla::psm::SSLErrorMessageType errorMessageType);
 
   /* Set SSL Status values */
--- a/toolkit/components/extensions/webrequest/ChannelWrapper.cpp
+++ b/toolkit/components/extensions/webrequest/ChannelWrapper.cpp
@@ -7,17 +7,21 @@
 #include "ChannelWrapper.h"
 
 #include "jsapi.h"
 #include "xpcpublic.h"
 
 #include "mozilla/BasePrincipal.h"
 #include "SystemPrincipal.h"
 
+#include "NSSErrorsService.h"
+#include "nsITransportSecurityInfo.h"
+
 #include "mozilla/AddonManagerWebAPI.h"
+#include "mozilla/ErrorNames.h"
 #include "mozilla/ResultExtensions.h"
 #include "mozilla/Unused.h"
 #include "nsIContentPolicy.h"
 #include "nsIHttpChannelInternal.h"
 #include "nsIHttpHeaderVisitor.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsILoadContext.h"
@@ -655,16 +659,52 @@ ChannelWrapper::GetRemoteAddress(nsCStri
 {
   aRetVal.SetIsVoid(true);
   if (nsCOMPtr<nsIHttpChannelInternal> internal = QueryChannel()) {
     Unused << internal->GetRemoteAddress(aRetVal);
   }
 }
 
 /*****************************************************************************
+ * Error handling
+ *****************************************************************************/
+
+void
+ChannelWrapper::GetErrorString(nsString& aRetVal) const
+{
+  if (nsCOMPtr<nsIChannel> chan = MaybeChannel()) {
+    nsCOMPtr<nsISupports> securityInfo;
+    Unused << chan->GetSecurityInfo(getter_AddRefs(securityInfo));
+    if (nsCOMPtr<nsITransportSecurityInfo> tsi = do_QueryInterface(securityInfo)) {
+      auto errorCode = tsi->GetErrorCode();
+      if (psm::IsNSSErrorCode(errorCode)) {
+        nsCOMPtr<nsINSSErrorsService> nsserr =
+          do_GetService(NS_NSS_ERRORS_SERVICE_CONTRACTID);
+
+        nsresult rv = psm::GetXPCOMFromNSSError(errorCode);
+        if (nsserr && NS_SUCCEEDED(nsserr->GetErrorMessage(rv, aRetVal))) {
+          return;
+        }
+      }
+    }
+
+    nsresult status;
+    if (NS_SUCCEEDED(chan->GetStatus(&status)) && NS_FAILED(status)) {
+      nsAutoCString name;
+      GetErrorName(status, name);
+      AppendUTF8toUTF16(name, aRetVal);
+    } else {
+      aRetVal.SetIsVoid(true);
+    }
+  } else {
+    aRetVal.AssignLiteral("NS_ERROR_UNEXPECTED");
+  }
+}
+
+/*****************************************************************************
  * Glue
  *****************************************************************************/
 
 JSObject*
 ChannelWrapper::WrapObject(JSContext* aCx, HandleObject aGivenProto)
 {
   return ChannelWrapperBinding::Wrap(aCx, this, aGivenProto);
 }
--- a/toolkit/components/extensions/webrequest/ChannelWrapper.h
+++ b/toolkit/components/extensions/webrequest/ChannelWrapper.h
@@ -129,16 +129,18 @@ public:
 
   dom::MozContentPolicyType Type() const;
 
 
   uint32_t StatusCode() const;
 
   void GetStatusLine(nsCString& aRetVal) const;
 
+  void GetErrorString(nsString& aRetVal) const;
+
 
   already_AddRefed<nsIURI> GetFinalURI(ErrorResult& aRv) const;
 
   void GetFinalURL(nsCString& aRetVal, ErrorResult& aRv) const;
 
 
   already_AddRefed<nsILoadInfo> GetLoadInfo() const
   {
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -15,20 +15,16 @@ 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.defineLazyServiceGetter(this, "webReqService",
@@ -675,45 +671,23 @@ HttpObserverManager = {
 
     if (filter.types && !filter.types.includes(policyType)) {
       return false;
     }
 
     return !filter.urls || filter.urls.matches(uri);
   },
 
-  get resultsMap() {
-    delete this.resultsMap;
-    this.resultsMap = new Map(Object.keys(Cr).map(name => [Cr[name], name]));
-    return this.resultsMap;
-  },
-
-  maybeError({channel}) {
-    // FIXME: Move to ChannelWrapper.
-
-    let {securityInfo} = channel;
-    if (securityInfo instanceof Ci.nsITransportSecurityInfo) {
-      if (NSSErrorsService.isNSSErrorCode(securityInfo.errorCode)) {
-        let nsresult = NSSErrorsService.getXPCOMFromNSSError(securityInfo.errorCode);
-        return {error: NSSErrorsService.getErrorMessage(nsresult)};
-      }
+  errorCheck(channel) {
+    let error = channel.errorString;
+    if (error) {
+      this.runChannelListener(channel, "onError", {error});
+      return true;
     }
-
-    if (!Components.isSuccessCode(channel.status)) {
-      return {error: this.resultsMap.get(channel.status) || "NS_ERROR_NET_UNKNOWN"};
-    }
-    return null;
-  },
-
-  errorCheck(channel) {
-    let errorData = this.maybeError(channel);
-    if (errorData) {
-      this.runChannelListener(channel, "onError", errorData);
-    }
-    return errorData;
+    return false;
   },
 
   getRequestData(channel, extraData) {
     let data = {
       requestId: String(channel.id),
       url: channel.finalURL,
       URI: channel.finalURI,
       method: channel.method,