Bug 1100024: Don't call Connect if the principal is on a local or remote blocklist (r=mcmanus)
authorMonica Chew <mmc@mozilla.com>
Fri, 09 Jan 2015 13:25:13 -0800
changeset 248898 671ad56e6e12927323e4d89d0076e7c04f58b55c
parent 248897 6c8be2f5b065ea8f72082b7d7cdbe53f0f53b9f6
child 248899 bfab295de6328219709e34060bf41c2c5af6916c
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1100024
milestone37.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 1100024: Don't call Connect if the principal is on a local or remote blocklist (r=mcmanus)
netwerk/base/src/nsBaseChannel.cpp
netwerk/base/src/nsChannelClassifier.cpp
netwerk/base/src/nsChannelClassifier.h
netwerk/protocol/http/HttpBaseChannel.cpp
netwerk/protocol/http/HttpBaseChannel.h
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
netwerk/protocol/http/nsIHttpChannelInternal.idl
--- a/netwerk/base/src/nsBaseChannel.cpp
+++ b/netwerk/base/src/nsBaseChannel.cpp
@@ -284,25 +284,20 @@ void
 nsBaseChannel::ClassifyURI()
 {
   // For channels created in the child process, delegate to the parent to
   // classify URIs.
   if (XRE_GetProcessType() != GeckoProcessType_Default) {
     return;
   }
 
-  nsresult rv;
-
   if (mLoadFlags & LOAD_CLASSIFY_URI) {
     nsRefPtr<nsChannelClassifier> classifier = new nsChannelClassifier();
     if (classifier) {
-      rv = classifier->Start(this);
-      if (NS_FAILED(rv)) {
-        Cancel(rv);
-      }
+      classifier->Start(this);
     } else {
       Cancel(NS_ERROR_OUT_OF_MEMORY);
     }
   }
 }
 
 //-----------------------------------------------------------------------------
 // nsBaseChannel::nsISupports
--- a/netwerk/base/src/nsChannelClassifier.cpp
+++ b/netwerk/base/src/nsChannelClassifier.cpp
@@ -44,17 +44,18 @@ static PRLogModuleInfo *gChannelClassifi
 #endif
 #undef LOG
 #define LOG(args)     PR_LOG(gChannelClassifierLog, PR_LOG_DEBUG, args)
 
 NS_IMPL_ISUPPORTS(nsChannelClassifier,
                   nsIURIClassifierCallback)
 
 nsChannelClassifier::nsChannelClassifier()
-  : mIsAllowListed(false)
+  : mIsAllowListed(false),
+    mSuspendedChannel(false)
 {
 #if defined(PR_LOGGING)
     if (!gChannelClassifierLog)
         gChannelClassifierLog = PR_NewLogModule("nsChannelClassifier");
 #endif
 }
 
 nsresult
@@ -204,71 +205,83 @@ nsChannelClassifier::NotifyTrackingProte
     doc->SetHasTrackingContentLoaded(true);
     securityUI->GetState(&state);
     state |= nsIWebProgressListener::STATE_LOADED_TRACKING_CONTENT;
     eventSink->OnSecurityChange(nullptr, state);
 
     return NS_OK;
 }
 
+void
+nsChannelClassifier::Start(nsIChannel *aChannel)
+{
+  mChannel = aChannel;
+  nsresult rv = StartInternal(aChannel);
+  if (NS_FAILED(rv)) {
+    // If we aren't getting a callback for any reason, assume a good verdict and
+    // make sure we resume the channel if necessary.
+    OnClassifyComplete(NS_OK);
+  }
+}
+
 nsresult
-nsChannelClassifier::Start(nsIChannel *aChannel)
+nsChannelClassifier::StartInternal(nsIChannel *aChannel)
 {
     // Should only be called in the parent process.
     MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
 
     // Don't bother to run the classifier on a load that has already failed.
     // (this might happen after a redirect)
     nsresult status;
     aChannel->GetStatus(&status);
     if (NS_FAILED(status))
-        return NS_OK;
+        return status;
 
     // Don't bother to run the classifier on a cached load that was
-    // previously classified.
+    // previously classified as good.
     if (HasBeenClassified(aChannel)) {
-        return NS_OK;
+        return NS_ERROR_UNEXPECTED;
     }
 
     nsCOMPtr<nsIURI> uri;
     nsresult rv = aChannel->GetURI(getter_AddRefs(uri));
     NS_ENSURE_SUCCESS(rv, rv);
 
     // Don't bother checking certain types of URIs.
     bool hasFlags;
     rv = NS_URIChainHasFlags(uri,
                              nsIProtocolHandler::URI_DANGEROUS_TO_LOAD,
                              &hasFlags);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (hasFlags) return NS_OK;
+    if (hasFlags) return NS_ERROR_UNEXPECTED;
 
     rv = NS_URIChainHasFlags(uri,
                              nsIProtocolHandler::URI_IS_LOCAL_FILE,
                              &hasFlags);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (hasFlags) return NS_OK;
+    if (hasFlags) return NS_ERROR_UNEXPECTED;
 
     rv = NS_URIChainHasFlags(uri,
                              nsIProtocolHandler::URI_IS_UI_RESOURCE,
                              &hasFlags);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (hasFlags) return NS_OK;
+    if (hasFlags) return NS_ERROR_UNEXPECTED;
 
     rv = NS_URIChainHasFlags(uri,
                              nsIProtocolHandler::URI_IS_LOCAL_RESOURCE,
                              &hasFlags);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (hasFlags) return NS_OK;
+    if (hasFlags) return NS_ERROR_UNEXPECTED;
 
     nsCOMPtr<nsIURIClassifier> uriClassifier =
         do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID, &rv);
     if (rv == NS_ERROR_FACTORY_NOT_REGISTERED ||
         rv == NS_ERROR_NOT_AVAILABLE) {
         // no URI classifier, ignore this failure.
-        return NS_OK;
+        return NS_ERROR_NOT_AVAILABLE;
     }
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIScriptSecurityManager> securityManager =
         do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIPrincipal> principal;
@@ -277,34 +290,38 @@ nsChannelClassifier::Start(nsIChannel *a
     NS_ENSURE_SUCCESS(rv, rv);
 
     bool expectCallback;
     bool trackingProtectionEnabled = false;
     (void)ShouldEnableTrackingProtection(aChannel, &trackingProtectionEnabled);
 
     rv = uriClassifier->Classify(principal, trackingProtectionEnabled, this,
                                  &expectCallback);
-    if (NS_FAILED(rv)) return rv;
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
 
     if (expectCallback) {
         // Suspend the channel, it will be resumed when we get the classifier
         // callback.
         rv = aChannel->Suspend();
         if (NS_FAILED(rv)) {
             // Some channels (including nsJSChannel) fail on Suspend.  This
             // shouldn't be fatal, but will prevent malware from being
             // blocked on these channels.
-            return NS_OK;
+            LOG(("nsChannelClassifier[%p]: Couldn't suspend channel", this));
+            return rv;
         }
 
-        mSuspendedChannel = aChannel;
-#ifdef DEBUG
+        mSuspendedChannel = true;
         LOG(("nsChannelClassifier[%p]: suspended channel %p",
-             this, mSuspendedChannel.get()));
-#endif
+             this, mChannel.get()));
+    } else {
+        LOG(("nsChannelClassifier[%p]: not expecting callback", this));
+        return NS_ERROR_FAILURE;
     }
 
     return NS_OK;
 }
 
 // Note in the cache entry that this URL was classified, so that future
 // cached loads don't need to be checked.
 void
@@ -313,18 +330,17 @@ nsChannelClassifier::MarkEntryClassified
     // Should only be called in the parent process.
     MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
 
     // Don't cache tracking classifications because we support allowlisting.
     if (status == NS_ERROR_TRACKING_URI || mIsAllowListed) {
         return;
     }
 
-    nsCOMPtr<nsICachingChannel> cachingChannel =
-        do_QueryInterface(mSuspendedChannel);
+    nsCOMPtr<nsICachingChannel> cachingChannel = do_QueryInterface(mChannel);
     if (!cachingChannel) {
         return;
     }
 
     nsCOMPtr<nsISupports> cacheToken;
     cachingChannel->GetCacheToken(getter_AddRefs(cacheToken));
     if (!cacheToken) {
         return;
@@ -439,41 +455,47 @@ nsChannelClassifier::SetBlockedTrackingC
 }
 
 NS_IMETHODIMP
 nsChannelClassifier::OnClassifyComplete(nsresult aErrorCode)
 {
     // Should only be called in the parent process.
     MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
 
+    LOG(("nsChannelClassifier[%p]:OnClassifyComplete", this));
     if (mSuspendedChannel) {
         MarkEntryClassified(aErrorCode);
 
         if (NS_FAILED(aErrorCode)) {
 #ifdef DEBUG
             nsCOMPtr<nsIURI> uri;
-            mSuspendedChannel->GetURI(getter_AddRefs(uri));
+            mChannel->GetURI(getter_AddRefs(uri));
             nsCString spec;
             uri->GetSpec(spec);
             LOG(("nsChannelClassifier[%p]: cancelling channel %p for %s "
-                 "with error code: %x", this, mSuspendedChannel.get(),
+                 "with error code: %x", this, mChannel.get(),
                  spec.get(), aErrorCode));
 #endif
 
             // Channel will be cancelled (page element blocked) due to tracking.
             // Do update the security state of the document and fire a security
             // change event.
             if (aErrorCode == NS_ERROR_TRACKING_URI) {
-              SetBlockedTrackingContent(mSuspendedChannel);
+              SetBlockedTrackingContent(mChannel);
             }
 
-            mSuspendedChannel->Cancel(aErrorCode);
-          }
-#ifdef DEBUG
+            mChannel->Cancel(aErrorCode);
+        }
         LOG(("nsChannelClassifier[%p]: resuming channel %p from "
-             "OnClassifyComplete", this, mSuspendedChannel.get()));
-#endif
-        mSuspendedChannel->Resume();
-        mSuspendedChannel = nullptr;
+             "OnClassifyComplete", this, mChannel.get()));
+        mChannel->Resume();
     }
+    nsresult rv;
+    nsCOMPtr<nsIHttpChannelInternal> channel = do_QueryInterface(mChannel, &rv);
+    // Even if we have cancelled the channel, we need to call
+    // ContinueBeginConnect so that we abort appropriately.
+    if (NS_SUCCEEDED(rv)) {
+        channel->ContinueBeginConnect();
+    }
+    mChannel = nullptr;
 
     return NS_OK;
 }
--- a/netwerk/base/src/nsChannelClassifier.h
+++ b/netwerk/base/src/nsChannelClassifier.h
@@ -15,26 +15,37 @@ class nsIChannel;
 class nsChannelClassifier MOZ_FINAL : public nsIURIClassifierCallback
 {
 public:
     nsChannelClassifier();
 
     NS_DECL_ISUPPORTS
     NS_DECL_NSIURICLASSIFIERCALLBACK
 
-    nsresult Start(nsIChannel *aChannel);
+    // Calls nsIURIClassifier.Classify with the principal of the given channel,
+    // and cancels the channel on a bad verdict.  If aChannel is
+    // nsIHttpChannelInternal, nsChannelClassifier must call
+    // ContinueBeginConnect once Start has successfully returned.
+    void Start(nsIChannel *aChannel);
 
 private:
-    nsCOMPtr<nsIChannel> mSuspendedChannel;
-    // Set true if the channel is on the allow list.
+    // True if the channel is on the allow list.
     bool mIsAllowListed;
+    // True if the channel has been suspended.
+    bool mSuspendedChannel;
+    nsCOMPtr<nsIChannel> mChannel;
 
     ~nsChannelClassifier() {}
+    // Caches good classifications for the channel principal.
     void MarkEntryClassified(nsresult status);
     bool HasBeenClassified(nsIChannel *aChannel);
+    // Helper function so that we ensure we call ContinueBeginConnect once
+    // Start is called. Returns NS_OK if and only if we will get a callback
+    // from the classifier service.
+    nsresult StartInternal(nsIChannel *aChannel);
     // Whether or not tracking protection should be enabled on this channel.
     nsresult ShouldEnableTrackingProtection(nsIChannel *aChannel, bool *result);
 
 public:
     // If we are blocking tracking content, update the corresponding flag in
     // the respective docshell and call nsISecurityEventSink::onSecurityChange.
     static nsresult SetBlockedTrackingContent(nsIChannel *channel);
     static nsresult NotifyTrackingProtectionDisabled(nsIChannel *aChannel);
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -1405,16 +1405,25 @@ HttpBaseChannel::RedirectTo(nsIURI *newU
   return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // HttpBaseChannel::nsIHttpChannelInternal
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
+HttpBaseChannel::ContinueBeginConnect()
+{
+  MOZ_ASSERT(XRE_GetProcessType() != GeckoProcessType_Default,
+             "The parent overrides this");
+  MOZ_ASSERT(false, "This method must be overridden");
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
 HttpBaseChannel::GetTopWindowURI(nsIURI **aTopWindowURI)
 {
   nsresult rv = NS_OK;
   nsCOMPtr<mozIThirdPartyUtil> util;
   // Only compute the top window URI once. In e10s, this must be computed in the
   // child. The parent gets the top window URI through HttpChannelOpenArgs.
   if (!mTopWindowURI) {
     util = do_GetService(THIRDPARTYUTIL_CONTRACTID);
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -183,16 +183,17 @@ public:
   NS_IMETHOD TakeAllSecurityMessages(nsCOMArray<nsISecurityConsoleMessage> &aMessages) MOZ_OVERRIDE;
   NS_IMETHOD GetResponseTimeoutEnabled(bool *aEnable) MOZ_OVERRIDE;
   NS_IMETHOD SetResponseTimeoutEnabled(bool aEnable) MOZ_OVERRIDE;
   NS_IMETHOD AddRedirect(nsIPrincipal *aRedirect) MOZ_OVERRIDE;
   NS_IMETHOD ForcePending(bool aForcePending) MOZ_OVERRIDE;
   NS_IMETHOD GetLastModifiedTime(PRTime* lastModifiedTime) MOZ_OVERRIDE;
   NS_IMETHOD ForceNoIntercept() MOZ_OVERRIDE;
   NS_IMETHOD GetTopWindowURI(nsIURI **aTopWindowURI) MOZ_OVERRIDE;
+  NS_IMETHOD ContinueBeginConnect();
 
   inline void CleanRedirectCacheChainIfNecessary()
   {
       mRedirectedCachekeys = nullptr;
   }
   NS_IMETHOD HTTPUpgrade(const nsACString & aProtocolName,
                          nsIHttpUpgradeListener *aListener) MOZ_OVERRIDE;
 
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -17,16 +17,17 @@
 #include "nsICacheEntry.h"
 #include "nsICryptoHash.h"
 #include "nsINetworkInterceptController.h"
 #include "nsIStringBundle.h"
 #include "nsIStreamListenerTee.h"
 #include "nsISeekableStream.h"
 #include "nsILoadGroupChild.h"
 #include "nsIProtocolProxyService2.h"
+#include "nsIURIClassifier.h"
 #include "nsMimeTypes.h"
 #include "nsNetUtil.h"
 #include "prprf.h"
 #include "prnetdb.h"
 #include "nsEscape.h"
 #include "nsStreamUtils.h"
 #include "nsIOService.h"
 #include "nsDNSPrefetch.h"
@@ -34,16 +35,17 @@
 #include "nsIRedirectResultListener.h"
 #include "mozilla/TimeStamp.h"
 #include "nsError.h"
 #include "nsPrintfCString.h"
 #include "nsAlgorithm.h"
 #include "GeckoProfiler.h"
 #include "nsIConsoleService.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/Preferences.h"
 #include "mozilla/VisualEventTracer.h"
 #include "nsISSLSocketControl.h"
 #include "sslt.h"
 #include "nsContentUtils.h"
 #include "nsIClassOfService.h"
 #include "nsIPermissionManager.h"
 #include "nsIPrincipal.h"
 #include "nsIScriptSecurityManager.h"
@@ -235,16 +237,17 @@ nsHttpChannel::nsHttpChannel()
     , mCacheEntryIsReadOnly(false)
     , mCacheEntryIsWriteOnly(false)
     , mCacheEntriesToWaitFor(0)
     , mHasQueryString(0)
     , mConcurentCacheAccess(0)
     , mIsPartialRequest(0)
     , mHasAutoRedirectVetoNotifier(0)
     , mPushedStream(nullptr)
+    , mLocalBlocklist(false)
     , mDidReval(false)
 {
     LOG(("Creating nsHttpChannel [this=%p]\n", this));
     mChannelCreationTime = PR_Now();
     mChannelCreationTimestamp = TimeStamp::Now();
 }
 
 nsHttpChannel::~nsHttpChannel()
@@ -321,17 +324,17 @@ nsHttpChannel::Connect()
     if (!net_IsValidHostName(nsDependentCString(mConnectionInfo->Host())))
         return NS_ERROR_UNKNOWN_HOST;
 
     // Finalize ConnectionInfo flags before SpeculativeConnect
     mConnectionInfo->SetAnonymous((mLoadFlags & LOAD_ANONYMOUS) != 0);
     mConnectionInfo->SetPrivate(mPrivateBrowsing);
     mConnectionInfo->SetNoSpdy(mCaps & NS_HTTP_DISALLOW_SPDY);
 
-    // Consider opening a TCP connection right away
+    // Consider opening a TCP connection right away.
     SpeculativeConnect();
 
     // Don't allow resuming when cache must be used
     if (mResuming && (mLoadFlags & LOAD_ONLY_FROM_CACHE)) {
         LOG(("Resuming from cache is not supported yet"));
         return NS_ERROR_DOCUMENT_NOT_CACHED;
     }
 
@@ -428,21 +431,21 @@ nsHttpChannel::ContinueConnect()
 }
 
 void
 nsHttpChannel::SpeculativeConnect()
 {
     // Before we take the latency hit of dealing with the cache, try and
     // get the TCP (and SSL) handshakes going so they can overlap.
 
-    // don't speculate on uses of the offline application cache,
-    // if we are offline, when doing http upgrade (i.e. websockets bootstrap),
-    // or if we can't do keep-alive (because then we couldn't reuse
-    // the speculative connection anyhow).
-    if (mApplicationCache || gIOService->IsOffline() ||
+    // don't speculate if we are on a local blocklist, on uses of the offline
+    // application cache, if we are offline, when doing http upgrade (i.e.
+    // websockets bootstrap), or if we can't do keep-alive (because then we
+    // couldn't reuse the speculative connection anyhow).
+    if (mLocalBlocklist || mApplicationCache || gIOService->IsOffline() ||
         mUpgradeProtocolCallback || !(mCaps & NS_HTTP_ALLOW_KEEPALIVE))
         return;
 
     // LOAD_ONLY_FROM_CACHE and LOAD_NO_NETWORK_IO must not hit network.
     // LOAD_FROM_CACHE and LOAD_CHECK_OFFLINE_CACHE are unlikely to hit network,
     // so skip preconnects for them.
     if (mLoadFlags & (LOAD_ONLY_FROM_CACHE | LOAD_FROM_CACHE |
                       LOAD_NO_NETWORK_IO | LOAD_CHECK_OFFLINE_CACHE))
@@ -4868,16 +4871,31 @@ nsHttpChannel::BeginConnect()
     // notify "http-on-modify-request" observers
     CallOnModifyRequestObservers();
 
     // Check to see if we should redirect this channel elsewhere by
     // nsIHttpChannel.redirectTo API request
     if (mAPIRedirectToURI) {
         return AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect);
     }
+    // Check to see if this principal exists on local blocklists.
+    if (mLoadFlags & LOAD_CLASSIFY_URI) {
+        nsCOMPtr<nsIURIClassifier> classifier = do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID);
+        if (classifier) {
+            nsCOMPtr<nsIPrincipal> principal = GetPrincipal(false);
+            bool tp = Preferences::GetBool("privacy.trackingprotection.enabled",
+                                           false);
+            nsresult response = NS_OK;
+            classifier->ClassifyLocal(principal, tp, &response);
+            if (NS_FAILED(response)) {
+                LOG(("nsHttpChannel::Found principal on local blocklist [this=%p]", this));
+                mLocalBlocklist = true;
+            }
+        }
+    }
 
     // If mTimingEnabled flag is not set after OnModifyRequest() then
     // clear the already recorded AsyncOpen value for consistency.
     if (!mTimingEnabled)
         mAsyncOpenTime = TimeStamp();
 
     // when proxying only use the pipeline bit if ProxyPipelining() allows it.
     if (!mConnectionInfo->UsingConnect() && mConnectionInfo->UsingHttpProxy()) {
@@ -4887,17 +4905,17 @@ nsHttpChannel::BeginConnect()
     }
 
     // if this somehow fails we can go on without it
     gHttpHandler->AddConnectionHeader(&mRequestHead.Headers(), mCaps);
 
     if (mLoadFlags & VALIDATE_ALWAYS || BYPASS_LOCAL_CACHE(mLoadFlags))
         mCaps |= NS_HTTP_REFRESH_DNS;
 
-    if (!mConnectionInfo->UsingHttpProxy() &&
+    if (!mLocalBlocklist && !mConnectionInfo->UsingHttpProxy() &&
         !(mLoadFlags & (LOAD_NO_NETWORK_IO | LOAD_ONLY_FROM_CACHE))) {
         // Start a DNS lookup very early in case the real open is queued the DNS can
         // happen in parallel. Do not do so in the presence of an HTTP proxy as
         // all lookups other than for the proxy itself are done by the proxy.
         // Also we don't do a lookup if the LOAD_NO_NETWORK_IO or
         // LOAD_ONLY_FROM_CACHE flags are set.
         //
         // We keep the DNS prefetch object around so that we can retrieve
@@ -4931,37 +4949,27 @@ nsHttpChannel::BeginConnect()
     // Force-Reload should reset the persistent connection pool for this host
     if (mLoadFlags & LOAD_FRESH_CONNECTION) {
         // just the initial document resets the whole pool
         if (mLoadFlags & LOAD_INITIAL_DOCUMENT_URI) {
             gHttpHandler->ConnMgr()->DoShiftReloadConnectionCleanup(mConnectionInfo);
         }
         mCaps &= ~NS_HTTP_ALLOW_PIPELINING;
     }
-
-    // We may have been cancelled already, either by on-modify-request
-    // listeners or by load group observers; in that case, we should
-    // not send the request to the server
-    if (mCanceled)
-        rv = mStatus;
-    else
-        rv = Connect();
-    if (NS_FAILED(rv)) {
-        LOG(("Calling AsyncAbort [rv=%x mCanceled=%i]\n", rv, mCanceled));
-        CloseCacheEntry(true);
-        AsyncAbort(rv);
-    } else if (mLoadFlags & LOAD_CLASSIFY_URI) {
-        nsRefPtr<nsChannelClassifier> classifier = new nsChannelClassifier();
-        rv = classifier->Start(this);
-        if (NS_FAILED(rv)) {
-            Cancel(rv);
-            return rv;
-        }
-    }
-
+    if (mCanceled || !mLocalBlocklist) {
+        return ContinueBeginConnect();
+    }
+    MOZ_ASSERT(!mCanceled && mLocalBlocklist);
+    // nsChannelClassifier must call ContinueBeginConnect after optionally
+    // cancelling the channel once we have a remote verdict. We call a concrete
+    // class instead of an nsI* that might be overridden.
+    nsRefPtr<nsChannelClassifier> classifier = new nsChannelClassifier();
+    LOG(("nsHttpChannel::Starting nsChannelClassifier %p [this=%p]",
+         classifier.get(), this));
+    classifier->Start(this);
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsIHttpChannelInternal
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
@@ -4989,16 +4997,40 @@ nsHttpChannel::SetPriority(int32_t value
         return NS_OK;
     mPriority = newValue;
     if (mTransaction)
         gHttpHandler->RescheduleTransaction(mTransaction, mPriority);
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
+// nsHttpChannel::nsIHttpChannelInternal
+//-----------------------------------------------------------------------------
+NS_IMETHODIMP
+nsHttpChannel::ContinueBeginConnect()
+{
+    LOG(("nsHttpChannel::ContinueBeginConnect [this=%p]", this));
+    nsresult rv;
+    // We may have been cancelled already, either by on-modify-request
+    // listeners or load group observers or nsChannelClassifier; in that case,
+    // we should not send the request to the server
+    if (mCanceled) {
+        rv = mStatus;
+    } else {
+        rv = Connect();
+    }
+    if (NS_FAILED(rv)) {
+        LOG(("Calling AsyncAbort [rv=%x mCanceled=%i]\n", rv, mCanceled));
+        CloseCacheEntry(true);
+        AsyncAbort(rv);
+    }
+    return rv;
+}
+
+//-----------------------------------------------------------------------------
 // HttpChannel::nsIClassOfService
 //-----------------------------------------------------------------------------
 NS_IMETHODIMP
 nsHttpChannel::SetClassFlags(uint32_t inFlags)
 {
     mClassOfService = inFlags;
     return NS_OK;
 }
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -114,16 +114,17 @@ public:
     NS_IMETHOD Cancel(nsresult status) MOZ_OVERRIDE;
     NS_IMETHOD Suspend() MOZ_OVERRIDE;
     NS_IMETHOD Resume() MOZ_OVERRIDE;
     // nsIChannel
     NS_IMETHOD GetSecurityInfo(nsISupports **aSecurityInfo) MOZ_OVERRIDE;
     NS_IMETHOD AsyncOpen(nsIStreamListener *listener, nsISupports *aContext) MOZ_OVERRIDE;
     // nsIHttpChannelInternal
     NS_IMETHOD SetupFallbackChannel(const char *aFallbackKey) MOZ_OVERRIDE;
+    NS_IMETHOD ContinueBeginConnect();
     // nsISupportsPriority
     NS_IMETHOD SetPriority(int32_t value) MOZ_OVERRIDE;
     // nsIClassOfService
     NS_IMETHOD SetClassFlags(uint32_t inFlags) MOZ_OVERRIDE;
     NS_IMETHOD AddClassFlags(uint32_t inFlags) MOZ_OVERRIDE;
     NS_IMETHOD ClearClassFlags(uint32_t inFlags) MOZ_OVERRIDE;
 
     // nsIResumableChannel
@@ -460,16 +461,19 @@ private:
     uint32_t                          mHasAutoRedirectVetoNotifier : 1;
 
     nsTArray<nsContinueRedirectionFunc> mRedirectFuncStack;
 
     // Needed for accurate DNS timing
     nsRefPtr<nsDNSPrefetch>           mDNSPrefetch;
 
     Http2PushedStream                 *mPushedStream;
+    // True if the channel's principal was found on a phishing, malware, or
+    // tracking (if tracking protection is enabled) blocklist
+    bool                              mLocalBlocklist;
 
     nsresult WaitForRedirectCallback();
     void PushRedirectAsyncFunc(nsContinueRedirectionFunc func);
     void PopRedirectAsyncFunc(nsContinueRedirectionFunc func);
 
     nsCString mUsername;
 
 protected:
--- a/netwerk/protocol/http/nsIHttpChannelInternal.idl
+++ b/netwerk/protocol/http/nsIHttpChannelInternal.idl
@@ -219,9 +219,15 @@ interface nsIHttpChannelInternal : nsISu
      * interception and proceed immediately to the network/cache.
      */
     void forceNoIntercept();
 
     /**
      * The URI of the top-level window that's associated with this channel.
      */
     readonly attribute nsIURI topWindowURI;
+
+    /**
+     * Used only by nsChannelClassifier to resume connecting or abort the
+     * channel after a remote classification verdict.
+     */
+    void continueBeginConnect();
 };