Bug 1521729 - P1: Support to get the result from NS_ShouldSecureUpgrade asynchronously r=mayhemer
authorKershaw Chang <kershaw@mozilla.com>
Mon, 18 Mar 2019 15:48:21 +0000
changeset 464833 917f02e20ab6edac88ba0d41d24a3b65409becdd
parent 464832 c0996caf797f1582c8797b991190e9bd3e53fb4f
child 464834 70b9b6dbe5b8e2b55a240aa3b39ff6087de92276
push id35727
push userdvarga@mozilla.com
push dateTue, 19 Mar 2019 09:48:59 +0000
treeherdermozilla-central@70baa37ae1eb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1521729
milestone68.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 1521729 - P1: Support to get the result from NS_ShouldSecureUpgrade asynchronously r=mayhemer 1. Make nsHttpChannel::OnBeforeConnect async. 2. There are two ways to get the result from NS_ShouldSecureUpgrade. One is getting the result synchronously from the input reference and the other is via the provided callback. Note that the callback is only used when the storage is not ready to read at startup. Differential Revision: https://phabricator.services.mozilla.com/D20191
netwerk/base/nsNetUtil.cpp
netwerk/base/nsNetUtil.h
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
--- a/netwerk/base/nsNetUtil.cpp
+++ b/netwerk/base/nsNetUtil.cpp
@@ -2525,21 +2525,24 @@ bool NS_IsSrcdocChannel(nsIChannel *aCha
     nsresult rv = vsc->GetIsSrcdocChannel(&isSrcdoc);
     if (NS_SUCCEEDED(rv)) {
       return isSrcdoc;
     }
   }
   return false;
 }
 
-nsresult NS_ShouldSecureUpgrade(nsIURI *aURI, nsILoadInfo *aLoadInfo,
-                                nsIPrincipal *aChannelResultPrincipal,
-                                bool aPrivateBrowsing, bool aAllowSTS,
-                                const OriginAttributes &aOriginAttributes,
-                                bool &aShouldUpgrade) {
+nsresult NS_ShouldSecureUpgrade(
+    nsIURI *aURI, nsILoadInfo *aLoadInfo, nsIPrincipal *aChannelResultPrincipal,
+    bool aPrivateBrowsing, bool aAllowSTS,
+    const OriginAttributes &aOriginAttributes, bool &aShouldUpgrade,
+    std::function<void(bool, nsresult)> &&aResultCallback,
+    bool &aWillCallback) {
+  aWillCallback = false;
+
   // Even if we're in private browsing mode, we still enforce existing STS
   // data (it is read-only).
   // if the connection is not using SSL and either the exact host matches or
   // a superdomain wants to force HTTPS, do it.
   bool isHttps = false;
   nsresult rv = aURI->SchemeIs("https", &isHttps);
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -2606,55 +2609,98 @@ nsresult NS_ShouldSecureUpgrade(nsIURI *
     // enforce Strict-Transport-Security
     nsISiteSecurityService *sss = gHttpHandler->GetSSService();
     NS_ENSURE_TRUE(sss, NS_ERROR_OUT_OF_MEMORY);
 
     bool isStsHost = false;
     uint32_t hstsSource = 0;
     uint32_t flags =
         aPrivateBrowsing ? nsISocketProvider::NO_PERMANENT_STORAGE : 0;
+
+    auto handleResultFunc = [aAllowSTS](bool aIsStsHost, uint32_t aHstsSource) {
+      if (aIsStsHost) {
+        LOG(("nsHttpChannel::Connect() STS permissions found\n"));
+        if (aAllowSTS) {
+          Telemetry::AccumulateCategorical(
+              Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::STS);
+          switch (aHstsSource) {
+            case nsISiteSecurityService::SOURCE_PRELOAD_LIST:
+              Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 0);
+              break;
+            case nsISiteSecurityService::SOURCE_ORGANIC_REQUEST:
+              Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 1);
+              break;
+            case nsISiteSecurityService::SOURCE_UNKNOWN:
+            default:
+              // record this as an organic request
+              Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 1);
+              break;
+          }
+          return true;
+        }
+        Telemetry::AccumulateCategorical(
+            Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::PrefBlockedSTS);
+      } else {
+        Telemetry::AccumulateCategorical(
+            Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::NoReasonToUpgrade);
+      }
+      return false;
+    };
+
+    // Calling |IsSecureURI| before the storage is ready to read will
+    // block the main thread. Once the storage is ready, we can call it
+    // from main thread.
+    static Atomic<bool, Relaxed> storageReady(false);
+    if (!storageReady && gSocketTransportService && aResultCallback) {
+      nsCOMPtr<nsIURI> uri = aURI;
+      nsCOMPtr<nsISiteSecurityService> service = sss;
+      rv = gSocketTransportService->Dispatch(
+          NS_NewRunnableFunction(
+              "net::NS_ShouldSecureUpgrade",
+              [service{std::move(service)}, uri{std::move(uri)}, flags(flags),
+               originAttributes(aOriginAttributes),
+               handleResultFunc{std::move(handleResultFunc)},
+               resultCallback{std::move(aResultCallback)}]() {
+                uint32_t hstsSource = 0;
+                bool isStsHost = false;
+                nsresult rv = service->IsSecureURI(
+                    nsISiteSecurityService::HEADER_HSTS, uri, flags,
+                    originAttributes, nullptr, &hstsSource, &isStsHost);
+
+                // Successfully get the result from |IsSecureURI| implies that
+                // the storage is ready to read.
+                storageReady = NS_SUCCEEDED(rv);
+                bool shouldUpgrade = handleResultFunc(isStsHost, hstsSource);
+
+                NS_DispatchToMainThread(NS_NewRunnableFunction(
+                    "net::NS_ShouldSecureUpgrade::ResultCallback",
+                    [rv, shouldUpgrade,
+                     resultCallback{std::move(resultCallback)}]() {
+                      resultCallback(shouldUpgrade, rv);
+                    }));
+              }),
+          NS_DISPATCH_NORMAL);
+      aWillCallback = NS_SUCCEEDED(rv);
+      return rv;
+    }
+
     rv = sss->IsSecureURI(nsISiteSecurityService::HEADER_HSTS, aURI, flags,
                           aOriginAttributes, nullptr, &hstsSource, &isStsHost);
 
     // if the SSS check fails, it's likely because this load is on a
     // malformed URI or something else in the setup is wrong, so any error
     // should be reported.
     NS_ENSURE_SUCCESS(rv, rv);
 
-    if (isStsHost) {
-      LOG(("nsHttpChannel::Connect() STS permissions found\n"));
-      if (aAllowSTS) {
-        Telemetry::AccumulateCategorical(
-            Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::STS);
-        aShouldUpgrade = true;
-        switch (hstsSource) {
-          case nsISiteSecurityService::SOURCE_PRELOAD_LIST:
-            Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 0);
-            break;
-          case nsISiteSecurityService::SOURCE_ORGANIC_REQUEST:
-            Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 1);
-            break;
-          case nsISiteSecurityService::SOURCE_UNKNOWN:
-          default:
-            // record this as an organic request
-            Telemetry::Accumulate(Telemetry::HSTS_UPGRADE_SOURCE, 1);
-            break;
-        }
-        return NS_OK;
-      }
-      Telemetry::AccumulateCategorical(
-          Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::PrefBlockedSTS);
-    } else {
-      Telemetry::AccumulateCategorical(
-          Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::NoReasonToUpgrade);
-    }
-  } else {
-    Telemetry::AccumulateCategorical(
-        Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::AlreadyHTTPS);
+    aShouldUpgrade = handleResultFunc(isStsHost, hstsSource);
+    return NS_OK;
   }
+
+  Telemetry::AccumulateCategorical(
+      Telemetry::LABELS_HTTP_SCHEME_UPGRADE_TYPE::AlreadyHTTPS);
   aShouldUpgrade = false;
   return NS_OK;
 }
 
 nsresult NS_GetSecureUpgradedURI(nsIURI *aURI, nsIURI **aUpgradedURI) {
   NS_MutateURI mutator(aURI);
   mutator.SetScheme(
       NS_LITERAL_CSTRING("https"));  // Change the scheme to HTTPS:
--- a/netwerk/base/nsNetUtil.h
+++ b/netwerk/base/nsNetUtil.h
@@ -2,16 +2,17 @@
 /* vim:set ts=4 sw=2 sts=2 et cin: */
 /* 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/. */
 
 #ifndef nsNetUtil_h__
 #define nsNetUtil_h__
 
+#include <functional>
 #include "mozilla/Maybe.h"
 #include "nsCOMPtr.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsILoadGroup.h"
 #include "nsINestedURI.h"
 #include "nsINetUtil.h"
 #include "nsIRequest.h"
@@ -885,21 +886,27 @@ bool NS_IsValidHTTPToken(const nsACStrin
 
 /**
  * Strip the leading or trailing HTTP whitespace per fetch spec section 2.2.
  */
 void NS_TrimHTTPWhitespace(const nsACString &aSource, nsACString &aDest);
 
 /**
  * Return true if the given request must be upgraded to HTTPS.
+ * If |aResultCallback| is provided and the storage is not ready to read, the
+ * result will be sent back through the callback and |aWillCallback| will be
+ * true. Otherwiew, the result will be set to |aShouldUpgrade| and
+ * |aWillCallback| is false.
  */
 nsresult NS_ShouldSecureUpgrade(
     nsIURI *aURI, nsILoadInfo *aLoadInfo, nsIPrincipal *aChannelResultPrincipal,
     bool aPrivateBrowsing, bool aAllowSTS,
-    const mozilla::OriginAttributes &aOriginAttributes, bool &aShouldUpgrade);
+    const mozilla::OriginAttributes &aOriginAttributes, bool &aShouldUpgrade,
+    std::function<void(bool, nsresult)> &&aResultCallback,
+    bool &aWillCallback);
 
 /**
  * Returns an https URI for channels that need to go through secure upgrades.
  */
 nsresult NS_GetSecureUpgradedURI(nsIURI *aURI, nsIURI **aUpgradedURI);
 
 nsresult NS_CompareLoadInfoAndLoadContext(nsIChannel *aChannel);
 
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -3776,19 +3776,20 @@ bool HttpChannelChild::ShouldInterceptUR
   NS_ENSURE_SUCCESS(rv, false);
   nsCOMPtr<nsIPrincipal> resultPrincipal;
   if (!isHttps && mLoadInfo) {
     nsContentUtils::GetSecurityManager()->GetChannelResultPrincipal(
         this, getter_AddRefs(resultPrincipal));
   }
   OriginAttributes originAttributes;
   NS_ENSURE_TRUE(NS_GetOriginAttributes(this, originAttributes), false);
-  rv =
-      NS_ShouldSecureUpgrade(aURI, mLoadInfo, resultPrincipal, mPrivateBrowsing,
-                             mAllowSTS, originAttributes, aShouldUpgrade);
+  bool notused = false;
+  rv = NS_ShouldSecureUpgrade(aURI, mLoadInfo, resultPrincipal,
+                              mPrivateBrowsing, mAllowSTS, originAttributes,
+                              aShouldUpgrade, nullptr, notused);
   NS_ENSURE_SUCCESS(rv, false);
 
   nsCOMPtr<nsIURI> upgradedURI;
   if (aShouldUpgrade) {
     rv = NS_GetSecureUpgradedURI(aURI, getter_AddRefs(upgradedURI));
     NS_ENSURE_SUCCESS(rv, false);
   }
 
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -538,27 +538,61 @@ nsresult nsHttpChannel::OnBeforeConnect(
   }
   bool isHttp = false;
   rv = mURI->SchemeIs("http", &isHttp);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // At this point it is no longer possible to call
   // HttpBaseChannel::UpgradeToSecure.
   mUpgradableToSecure = false;
+  bool shouldUpgrade = mUpgradeToSecure;
   if (isHttp) {
-    bool shouldUpgrade = mUpgradeToSecure;
     if (!shouldUpgrade) {
+      RefPtr<nsHttpChannel> self = this;
+      auto resultCallback = [self{std::move(self)}](bool aResult,
+                                                    nsresult aStatus) {
+        nsresult rv = self->ContinueOnBeforeConnect(aResult, aStatus);
+        if (NS_FAILED(rv)) {
+          self->CloseCacheEntry(false);
+          Unused << self->AsyncAbort(rv);
+        }
+      };
+
+      bool willCallback = false;
       rv = NS_ShouldSecureUpgrade(mURI, mLoadInfo, resultPrincipal,
                                   mPrivateBrowsing, mAllowSTS, originAttributes,
-                                  shouldUpgrade);
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-    if (shouldUpgrade) {
-      return AsyncCall(&nsHttpChannel::HandleAsyncRedirectChannelToHttps);
-    }
+                                  shouldUpgrade, std::move(resultCallback),
+                                  willCallback);
+      LOG(
+          ("nsHttpChannel::OnBeforeConnect "
+           "[this=%p willCallback=%d rv=%" PRIx32 "]\n",
+           this, willCallback, static_cast<uint32_t>(rv)));
+
+      if (NS_FAILED(rv) || MOZ_UNLIKELY(willCallback)) {
+        return rv;
+      }
+    }
+  }
+
+  return ContinueOnBeforeConnect(shouldUpgrade, NS_OK);
+}
+
+nsresult nsHttpChannel::ContinueOnBeforeConnect(bool aShouldUpgrade,
+                                                nsresult aStatus) {
+  LOG(
+      ("nsHttpChannel::ContinueOnBeforeConnect "
+       "[this=%p aShouldUpgrade=%d rv=%" PRIx32 "]\n",
+       this, aShouldUpgrade, static_cast<uint32_t>(aStatus)));
+
+  if (NS_FAILED(aStatus)) {
+    return aStatus;
+  }
+
+  if (aShouldUpgrade) {
+    return AsyncCall(&nsHttpChannel::HandleAsyncRedirectChannelToHttps);
   }
 
   // ensure that we are using a valid hostname
   if (!net_IsValidHostName(nsDependentCString(mConnectionInfo->Origin())))
     return NS_ERROR_UNKNOWN_HOST;
 
   if (mUpgradeProtocolCallback) {
     // Websockets can run over HTTP/2, but other upgrades can't.
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -324,16 +324,18 @@ class nsHttpChannel final : public HttpB
   // is required, this funciton will just return NS_OK and BeginConnectActual()
   // will be called when callback. See Bug 1325054 for more information.
   nsresult BeginConnect();
   MOZ_MUST_USE nsresult ContinueBeginConnectWithResult();
   void ContinueBeginConnect();
   MOZ_MUST_USE nsresult PrepareToConnect();
   void HandleOnBeforeConnect();
   MOZ_MUST_USE nsresult OnBeforeConnect();
+  MOZ_MUST_USE nsresult ContinueOnBeforeConnect(bool aShouldUpgrade,
+                                                nsresult aStatus);
   void OnBeforeConnectContinue();
   MOZ_MUST_USE nsresult Connect();
   void SpeculativeConnect();
   MOZ_MUST_USE nsresult SetupTransaction();
   MOZ_MUST_USE nsresult CallOnStartRequest();
   MOZ_MUST_USE nsresult ProcessResponse();
   void AsyncContinueProcessResponse();
   MOZ_MUST_USE nsresult ContinueProcessResponse1();