Bug 1325054 - Defer any possible connection establishment in BeginConnect until knowing if it's a tracker. draft
authorHenry Chang <hchang@mozilla.com>
Mon, 06 Mar 2017 17:43:11 +0800
changeset 494465 b9fd90bd6babb621aff226892a57012ce0f9d6d4
parent 494263 b7e42143bbbc9dc3e5c05bd1e93b6485ce1d0ad4
child 548099 3e6bb525f4436f3598b9191cfdc64acf08c0c2ff
push id48030
push userhchang@mozilla.com
push dateTue, 07 Mar 2017 03:43:59 +0000
bugs1325054
milestone55.0a1
Bug 1325054 - Defer any possible connection establishment in BeginConnect until knowing if it's a tracker. MozReview-Commit-ID: 59MzYAVlr6i
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -5788,16 +5788,94 @@ nsHttpChannel::AsyncOpen(nsIStreamListen
     if (NS_FAILED(rv)) {
         CloseCacheEntry(false);
         AsyncAbort(rv);
     }
 
     return NS_OK;
 }
 
+namespace {
+
+class InitLocalBlockListXpcCallback final : public nsIURIClassifierCallback {
+public:
+  using CallbackType = nsHttpChannel::InitLocalBlockListCallback;
+
+  explicit InitLocalBlockListXpcCallback(const CallbackType& aCallback)
+    : mCallback(aCallback)
+  {
+  }
+
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIURICLASSIFIERCALLBACK
+
+private:
+  ~InitLocalBlockListXpcCallback() = default;
+
+  CallbackType mCallback;
+};
+
+NS_IMPL_ISUPPORTS(InitLocalBlockListXpcCallback, nsIURIClassifierCallback)
+
+/*virtual*/ nsresult
+InitLocalBlockListXpcCallback::OnClassifyComplete(nsresult /*aErrorCode*/,
+                                               const nsACString& aLists, // Only this matters.
+                                               const nsACString& /*aProvider*/,
+                                               const nsACString& /*aPrefix*/)
+{
+    bool localBlockList = !aLists.IsEmpty();
+    mCallback(localBlockList);
+    return NS_OK;
+}
+
+} // end of unnamed namespace/
+
+void
+nsHttpChannel::InitLocalBlockList(const InitLocalBlockListCallback& aCallback)
+{
+    if (!(mLoadFlags & LOAD_CLASSIFY_URI)) {
+        aCallback(false);
+        return;
+    }
+
+    // Check to see if this principal exists on local blocklists.
+    nsCOMPtr<nsIURIClassifier> classifier = do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID);
+    RefPtr<nsChannelClassifier> channelClassifier = new nsChannelClassifier(this);
+    bool tpEnabled = false;
+    channelClassifier->ShouldEnableTrackingProtection(&tpEnabled);
+    if (!classifier || !tpEnabled) {
+        aCallback(false);
+        return;
+    }
+
+    // We skip speculative connections by setting mLocalBlocklist only
+    // when tracking protection is enabled. Though we could do this for
+    // both phishing and malware, it is not necessary for correctness,
+    // since no network events will be received while the
+    // nsChannelClassifier is in progress. See bug 1122691.
+    nsCOMPtr<nsIURI> uri;
+    nsresult rv = GetURI(getter_AddRefs(uri));
+    if (NS_FAILED(rv) || !uri) {
+        aCallback(false);
+        return;
+    }
+
+    nsAutoCString tables;
+    Preferences::GetCString("urlclassifier.trackingTable", &tables);
+    nsTArray<nsCString> results;
+
+    RefPtr<InitLocalBlockListXpcCallback> xpcCallback
+        = new InitLocalBlockListXpcCallback(aCallback);
+    rv = classifier->AsyncClassifyLocalWithTables(uri, tables, xpcCallback);
+    if (NS_FAILED(rv)) {
+        aCallback(false);
+        return;
+    }
+}
+
 NS_IMETHODIMP
 nsHttpChannel::AsyncOpen2(nsIStreamListener *aListener)
 {
   nsCOMPtr<nsIStreamListener> listener = aListener;
   nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
   if (NS_WARN_IF(NS_FAILED(rv))) {
       ReleaseListeners();
       return rv;
@@ -5806,16 +5884,41 @@ nsHttpChannel::AsyncOpen2(nsIStreamListe
 }
 
 // BeginConnect() SHOULD NOT call AsyncAbort(). AsyncAbort will be called by
 // functions that called BeginConnect if needed. Only AsyncOpen and
 // OnProxyAvailable ever call BeginConnect.
 nsresult
 nsHttpChannel::BeginConnect()
 {
+    nsresult rv = BeginConnectWouldSucceed();
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
+
+    if(mAPIRedirectToURI) {
+        return AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect);
+    }
+
+    RefPtr<nsHttpChannel> self = this;
+    InitLocalBlockList([self](bool aLocalBlockList) -> void  {
+        self->mLocalBlocklist = aLocalBlockList;
+        nsresult rv = self->BeginConnectActual();
+        if (NS_FAILED(rv)) {
+          self->CloseCacheEntry(false);
+          self->AsyncAbort(rv);
+        }
+    });
+
+    return NS_OK;
+}
+
+nsresult
+nsHttpChannel::BeginConnectWouldSucceed()
+{
     LOG(("nsHttpChannel::BeginConnect [this=%p]\n", this));
     nsresult rv;
 
     // Construct connection info object
     nsAutoCString host;
     nsAutoCString scheme;
     int32_t port = -1;
     bool isHttps = false;
@@ -5935,87 +6038,30 @@ nsHttpChannel::BeginConnect()
     // notify "http-on-modify-request" observers
     CallOnModifyRequestObservers();
 
     SetLoadGroupUserAgentOverride();
 
     // 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.
-    RefPtr<nsChannelClassifier> channelClassifier = new nsChannelClassifier(this);
-    if (mLoadFlags & LOAD_CLASSIFY_URI) {
-        nsCOMPtr<nsIURIClassifier> classifier = do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID);
-        bool tpEnabled = false;
-        channelClassifier->ShouldEnableTrackingProtection(&tpEnabled);
-        if (classifier && tpEnabled) {
-            // We skip speculative connections by setting mLocalBlocklist only
-            // when tracking protection is enabled. Though we could do this for
-            // both phishing and malware, it is not necessary for correctness,
-            // since no network events will be received while the
-            // nsChannelClassifier is in progress. See bug 1122691.
-
-            // We cannot check the entity whitelist here (IsTrackerWhitelisted())
-            // because that method is asynchronous and we need to run
-            // synchronously here.
-            // See https://bugzilla.mozilla.org/show_bug.cgi?id=1100024#c2.
-            nsCOMPtr<nsIURI> uri;
-            rv = GetURI(getter_AddRefs(uri));
-            if (NS_SUCCEEDED(rv) && uri) {
-                nsAutoCString tables;
-                Preferences::GetCString("urlclassifier.trackingTable", &tables);
-                nsTArray<nsCString> results;
-                rv = classifier->ClassifyLocalWithTables(uri, tables, results);
-                if (NS_SUCCEEDED(rv) && !results.IsEmpty()) {
-                    LOG(("nsHttpChannel::ClassifyLocalWithTables found "
-                         "uri on local tracking blocklist [this=%p]",
-                         this));
-                    mLocalBlocklist = true;
-                } else {
-                    LOG(("nsHttpChannel::ClassifyLocalWithTables no result "
-                         "found [this=%p]", this));
-                }
-            }
-        }
+        return NS_OK;
     }
 
     // If mTimingEnabled flag is not set after OnModifyRequest() then
     // clear the already recorded AsyncOpen value for consistency.
     if (!mTimingEnabled)
         mAsyncOpenTime = TimeStamp();
 
     // if this somehow fails we can go on without it
     gHttpHandler->AddConnectionHeader(&mRequestHead, mCaps);
 
     if (mLoadFlags & VALIDATE_ALWAYS || BYPASS_LOCAL_CACHE(mLoadFlags))
         mCaps |= NS_HTTP_REFRESH_DNS;
 
-    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
-        // timing information from it. There is no guarantee that we actually
-        // use the DNS prefetch data for the real connection, but as we keep
-        // this data around for 3 minutes by default, this should almost always
-        // be correct, and even when it isn't, the timing still represents _a_
-        // valid DNS lookup timing for the site, even if it is not _the_
-        // timing we used.
-        LOG(("nsHttpChannel::BeginConnect [this=%p] prefetching%s\n",
-             this, mCaps & NS_HTTP_REFRESH_DNS ? ", refresh requested" : ""));
-        mDNSPrefetch = new nsDNSPrefetch(mURI, this, mTimingEnabled);
-        mDNSPrefetch->PrefetchHigh(mCaps & NS_HTTP_REFRESH_DNS);
-    }
-
     // Adjust mCaps according to our request headers:
     //  - If "Connection: close" is set as a request header, then do not bother
     //    trying to establish a keep-alive connection.
     if (mRequestHead.HasHeaderValue(nsHttp::Connection, "close"))
         mCaps &= ~(NS_HTTP_ALLOW_KEEPALIVE);
 
     if (gHttpHandler->CriticalRequestPrioritization()) {
         if (mClassOfService & nsIClassOfService::Leader) {
@@ -6045,45 +6091,77 @@ nsHttpChannel::BeginConnect()
     if (mClassOfService & nsIClassOfService::Throttleable) {
         nsIThrottlingService *throttler = gHttpHandler->GetThrottlingService();
         if (throttler) {
             // This may immediately Suspend() this channel.
             throttler->AddChannel(this);
         }
     }
 
+    return NS_OK;
+}
+
+nsresult
+nsHttpChannel::BeginConnectActual()
+{
+    // Even though this can be done in BeginConnectWouldSucceed(), we'd like to
+    // assure no connection will never be tried to establish until
+    // |BeginConnectInternal|.
     if (!(mLoadFlags & LOAD_CLASSIFY_URI)) {
         return ContinueBeginConnectWithResult();
     }
 
+    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
+        // timing information from it. There is no guarantee that we actually
+        // use the DNS prefetch data for the real connection, but as we keep
+        // this data around for 3 minutes by default, this should almost always
+        // be correct, and even when it isn't, the timing still represents _a_
+        // valid DNS lookup timing for the site, even if it is not _the_
+        // timing we used.
+        LOG(("nsHttpChannel::BeginConnect [this=%p] prefetching%s\n",
+             this, mCaps & NS_HTTP_REFRESH_DNS ? ", refresh requested" : ""));
+        mDNSPrefetch = new nsDNSPrefetch(mURI, this, mTimingEnabled);
+        mDNSPrefetch->PrefetchHigh(mCaps & NS_HTTP_REFRESH_DNS);
+    }
+
     // mLocalBlocklist is true only if tracking protection is enabled and the
     // URI is a tracking domain, it makes no guarantees about phishing or
     // malware, so if LOAD_CLASSIFY_URI is true we must call
     // nsChannelClassifier to catch phishing and malware URIs.
     bool callContinueBeginConnect = true;
     if (!mLocalBlocklist) {
         // Here we call ContinueBeginConnectWithResult and not
         // ContinueBeginConnect so that in the case of an error we do not start
         // channelClassifier.
-        rv = ContinueBeginConnectWithResult();
+        nsresult rv = ContinueBeginConnectWithResult();
         if (NS_FAILED(rv)) {
             return rv;
         }
         callContinueBeginConnect = false;
     }
     // nsChannelClassifier calls ContinueBeginConnect if it has not already
     // been called, after optionally cancelling the channel once we have a
     // remote verdict. We call a concrete class instead of an nsI* that might
     // be overridden.
+    RefPtr<nsChannelClassifier> channelClassifier = new nsChannelClassifier(this);
     LOG(("nsHttpChannel::Starting nsChannelClassifier %p [this=%p]",
          channelClassifier.get(), this));
     channelClassifier->Start();
     if (callContinueBeginConnect) {
         return ContinueBeginConnectWithResult();
     }
+
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsHttpChannel::GetEncodedBodySize(uint64_t *aEncodedBodySize)
 {
     if (mCacheEntry && !mCacheEntryIsWriteOnly) {
         int64_t dataSize = 0;
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -179,16 +179,18 @@ public:
     nsresult AddSecurityMessage(const nsAString& aMessageTag,
                                 const nsAString& aMessageCategory) override;
 
     void SetWarningReporter(HttpChannelSecurityWarningReporter* aReporter)
       { mWarningReporter = aReporter; }
 
 public: /* internal necko use only */
 
+    using InitLocalBlockListCallback = std::function<void(bool)>;
+
     void InternalSetUploadStream(nsIInputStream *uploadStream)
       { mUploadStream = uploadStream; }
     void SetUploadStreamHasHeaders(bool hasHeaders)
       { mUploadStreamHasHeaders = hasHeaders; }
 
     nsresult SetReferrerWithPolicyInternal(nsIURI *referrer,
                                            uint32_t referrerPolicy) {
         nsAutoCString spec;
@@ -284,16 +286,20 @@ public:
 
 protected:
     virtual ~nsHttpChannel();
 
 private:
     typedef nsresult (nsHttpChannel::*nsContinueRedirectionFunc)(nsresult result);
 
     bool     RequestIsConditional();
+    // Might change internal states but will not make real connection.
+    nsresult BeginConnectWouldSucceed();
+    // Connection will only be established in this function.
+    nsresult BeginConnectActual();
     nsresult BeginConnect();
     nsresult ContinueBeginConnectWithResult();
     void     ContinueBeginConnect();
     nsresult Connect();
     void     SpeculativeConnect();
     nsresult SetupTransaction();
     void     SetupTransactionRequestContext();
     nsresult CallOnStartRequest();
@@ -317,16 +323,18 @@ private:
     nsresult EnsureAssocReq();
     void     ProcessSSLInformation();
     bool     IsHTTPS();
 
     nsresult ContinueOnStartRequest1(nsresult);
     nsresult ContinueOnStartRequest2(nsresult);
     nsresult ContinueOnStartRequest3(nsresult);
 
+    void InitLocalBlockList(const InitLocalBlockListCallback& aCallback);
+
     // redirection specific methods
     void     HandleAsyncRedirect();
     void     HandleAsyncAPIRedirect();
     nsresult ContinueHandleAsyncRedirect(nsresult);
     void     HandleAsyncNotModified();
     void     HandleAsyncFallback();
     nsresult ContinueHandleAsyncFallback(nsresult);
     nsresult PromptTempRedirect();