Bug 1341506 - Part 1: Implement and use nsIURIClassifier.asyncClassifyLocalWithTables. r=Ehsan,francois
authorHenry Chang <hchang@mozilla.com>
Wed, 01 Mar 2017 11:27:51 +0800
changeset 394345 399c4050991d8c599e59f9e56ee6e6a3efaaa4b1
parent 394344 2da1219f234788ee0d09565dd204628e6fa693f1
child 394346 927f95814b295434b0ceca5eca3c81c8ae36eff1
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan, francois
bugs1341506
milestone54.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 1341506 - Part 1: Implement and use nsIURIClassifier.asyncClassifyLocalWithTables. r=Ehsan,francois MozReview-Commit-ID: 8dvYM4o2Xxw
netwerk/base/nsChannelClassifier.cpp
netwerk/base/nsChannelClassifier.h
netwerk/base/nsIURIClassifier.idl
netwerk/protocol/http/nsHttpChannel.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/tests/mochitest/test_classifier.html
--- a/netwerk/base/nsChannelClassifier.cpp
+++ b/netwerk/base/nsChannelClassifier.cpp
@@ -659,18 +659,77 @@ nsChannelClassifier::SetBlockedContent(n
                                   doc,
                                   nsContentUtils::eNECKO_PROPERTIES,
                                   message,
                                   params, ArrayLength(params));
 
   return NS_OK;
 }
 
+namespace {
+
+class IsTrackerWhitelistedCallback final : public nsIURIClassifierCallback {
+public:
+  explicit IsTrackerWhitelistedCallback(nsChannelClassifier* aClosure,
+                                        const nsACString& aList,
+                                        const nsACString& aProvider,
+                                        const nsACString& aPrefix,
+                                        const nsACString& aWhitelistEntry)
+    : mClosure(aClosure)
+    , mWhitelistEntry(aWhitelistEntry)
+    , mList(aList)
+    , mProvider(aProvider)
+    , mPrefix(aPrefix)
+  {
+  }
+
+  NS_DECL_THREADSAFE_ISUPPORTS
+  NS_DECL_NSIURICLASSIFIERCALLBACK
+
+private:
+  ~IsTrackerWhitelistedCallback() = default;
+
+  RefPtr<nsChannelClassifier> mClosure;
+  nsCString mWhitelistEntry;
+
+  // The following 3 values are for forwarding the callback.
+  nsCString mList;
+  nsCString mProvider;
+  nsCString mPrefix;
+};
+
+NS_IMPL_ISUPPORTS(IsTrackerWhitelistedCallback, nsIURIClassifierCallback)
+
+
+/*virtual*/ nsresult
+IsTrackerWhitelistedCallback::OnClassifyComplete(nsresult /*aErrorCode*/,
+                                                 const nsACString& aLists, // Only this matters.
+                                                 const nsACString& /*aProvider*/,
+                                                 const nsACString& /*aPrefix*/)
+{
+  nsresult rv;
+  if (aLists.IsEmpty()) {
+    LOG(("nsChannelClassifier[%p]: %s is not in the whitelist",
+       mClosure.get(), mWhitelistEntry.get()));
+    rv = NS_ERROR_TRACKING_URI;
+  } else {
+    LOG(("nsChannelClassifier[%p]:OnClassifyComplete tracker found "
+         "in whitelist so we won't block it", mClosure.get()));
+    rv = NS_OK;
+  }
+
+  return mClosure->OnClassifyCompleteInternal(rv, mList, mProvider, mPrefix);
+}
+
+} // end of unnamed namespace/
+
 nsresult
-nsChannelClassifier::IsTrackerWhitelisted()
+nsChannelClassifier::IsTrackerWhitelisted(const nsACString& aList,
+                                          const nsACString& aProvider,
+                                          const nsACString& aPrefix)
 {
   nsresult rv;
   nsCOMPtr<nsIURIClassifier> uriClassifier =
     do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsAutoCString tables;
   Preferences::GetCString("urlclassifier.trackingWhitelistTable", &tables);
@@ -710,45 +769,48 @@ nsChannelClassifier::IsTrackerWhiteliste
     pageHostname + NS_LITERAL_CSTRING("/?resource=") + resourceDomain;
   LOG(("nsChannelClassifier[%p]: Looking for %s in the whitelist",
        this, whitelistEntry.get()));
 
   nsCOMPtr<nsIURI> whitelistURI;
   rv = NS_NewURI(getter_AddRefs(whitelistURI), whitelistEntry);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Check whether or not the tracker is in the entity whitelist
-  nsTArray<nsCString> results;
-  rv = uriClassifier->ClassifyLocalWithTables(whitelistURI, tables, results);
-  NS_ENSURE_SUCCESS(rv, rv);
-  if (!results.IsEmpty()) {
-    return NS_OK; // found it on the whitelist, must not be blocked
-  }
+  RefPtr<IsTrackerWhitelistedCallback> cb =
+    new IsTrackerWhitelistedCallback(this, aList, aProvider, aPrefix,
+                                     whitelistEntry);
 
-  LOG(("nsChannelClassifier[%p]: %s is not in the whitelist",
-       this, whitelistEntry.get()));
-  return NS_ERROR_TRACKING_URI;
+  return uriClassifier->AsyncClassifyLocalWithTables(whitelistURI, tables, cb);
 }
 
 NS_IMETHODIMP
 nsChannelClassifier::OnClassifyComplete(nsresult aErrorCode,
                                         const nsACString& aList,
                                         const nsACString& aProvider,
                                         const nsACString& aPrefix)
 {
-    // Should only be called in the parent process.
-    MOZ_ASSERT(XRE_IsParentProcess());
+  // Should only be called in the parent process.
+  MOZ_ASSERT(XRE_IsParentProcess());
+
+  if (aErrorCode == NS_ERROR_TRACKING_URI &&
+      NS_SUCCEEDED(IsTrackerWhitelisted(aList, aProvider, aPrefix))) {
+    // OnClassifyCompleteInternal() will be called once we know
+    // if the tracker is whitelisted.
+    return NS_OK;
+  }
 
-    if (aErrorCode == NS_ERROR_TRACKING_URI &&
-        NS_SUCCEEDED(IsTrackerWhitelisted())) {
-      LOG(("nsChannelClassifier[%p]:OnClassifyComplete tracker found "
-           "in whitelist so we won't block it", this));
-      aErrorCode = NS_OK;
-    }
+  return OnClassifyCompleteInternal(aErrorCode, aList, aProvider, aPrefix);
+}
 
+nsresult
+nsChannelClassifier::OnClassifyCompleteInternal(nsresult aErrorCode,
+                                                const nsACString& aList,
+                                                const nsACString& aProvider,
+                                                const nsACString& aPrefix)
+{
     if (mSuspendedChannel) {
       nsAutoCString errorName;
       if (LOG_ENABLED()) {
         GetErrorName(aErrorCode, errorName);
         LOG(("nsChannelClassifier[%p]:OnClassifyComplete %s (suspended channel)",
              this, errorName.get()));
       }
       MarkEntryClassified(aErrorCode);
--- a/netwerk/base/nsChannelClassifier.h
+++ b/netwerk/base/nsChannelClassifier.h
@@ -28,16 +28,23 @@ public:
     NS_DECL_NSIOBSERVER
 
     // Calls nsIURIClassifier.Classify with the principal of the given channel,
     // and cancels the channel on a bad verdict.
     void Start();
     // Whether or not tracking protection should be enabled on this channel.
     nsresult ShouldEnableTrackingProtection(bool *result);
 
+    // Called once we actually classified an URI. (An additional whitelist
+    // check will be done if the classifier reports the URI is a tracker.)
+    nsresult OnClassifyCompleteInternal(nsresult aErrorCode,
+                                        const nsACString& aList,
+                                        const nsACString& aProvider,
+                                        const nsACString& aPrefix);
+
 private:
     // True if the channel is on the allow list.
     bool mIsAllowListed;
     // True if the channel has been suspended.
     bool mSuspendedChannel;
     nsCOMPtr<nsIChannel> mChannel;
     Maybe<bool> mTrackingProtectionEnabled;
 
@@ -45,17 +52,19 @@ private:
     // 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();
     // Helper function to check a tracking URI against the whitelist
-    nsresult IsTrackerWhitelisted();
+    nsresult IsTrackerWhitelisted(const nsACString& aList,
+                                  const nsACString& aProvider,
+                                  const nsACString& aPrefix);
     // Helper function to check a URI against the hostname whitelist
     bool IsHostnameWhitelisted(nsIURI *aUri, const nsACString &aWhitelisted);
     // Checks that the channel was loaded by the URI currently loaded in aDoc
     static bool SameLoadingURI(nsIDocument *aDoc, nsIChannel *aChannel);
 
     nsresult ShouldEnableTrackingProtectionInternal(nsIChannel *aChannel,
                                                     bool *result);
 
--- a/netwerk/base/nsIURIClassifier.idl
+++ b/netwerk/base/nsIURIClassifier.idl
@@ -73,13 +73,24 @@ interface nsIURIClassifier : nsISupports
 
   /**
    * Synchronously classify a URI with a comma-separated string
    * containing the given tables. This does not make network requests.
    * The result is an array of table names that match.
    */
   [noscript] StringArrayRef classifyLocalWithTables(in nsIURI aURI, in ACString aTables);
   /**
+   * Asynchronously classify a URI with a comma-separated string
+   * containing the given tables. This does not make network requests.
+   * The callback does NOT totally follow nsIURIClassifierCallback's
+   * semantics described above. Only |aList| will be meaningful, which
+   * is a comma separated list of table names. (same as what classifyLocal
+   * returns.)
+   */
+  void asyncClassifyLocalWithTables(in nsIURI aURI,
+                                    in ACString aTables,
+                                    in nsIURIClassifierCallback aCallback);
+  /**
    * Same as above, but returns a comma separated list of table names.
    * This is an internal interface used only for testing purposes.
    */
   ACString classifyLocal(in nsIURI aURI, in ACString aTables);
 };
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -3038,17 +3038,17 @@ nsHttpChannel::EnsureAssocReq()
 
 bool
 nsHttpChannel::IsResumable(int64_t partialLen, int64_t contentLength,
                            bool ignoreMissingPartialLen) const
 {
     bool hasContentEncoding =
         mCachedResponseHead->HasHeader(nsHttp::Content_Encoding);
 
-    nsAutoCString etag; 
+    nsAutoCString etag;
     mCachedResponseHead->GetHeader(nsHttp::ETag, etag);
     bool hasWeakEtag = !etag.IsEmpty() &&
                        StringBeginsWith(etag, NS_LITERAL_CSTRING("W/"));
 
     return (partialLen < contentLength) &&
            (partialLen > 0 || ignoreMissingPartialLen) &&
            !hasContentEncoding && !hasWeakEtag &&
            mCachedResponseHead->IsResumable() &&
@@ -6112,16 +6112,21 @@ nsHttpChannel::BeginConnect()
         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()) {
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -49,16 +49,17 @@
 #include "nsIPrincipal.h"
 #include "Classifier.h"
 #include "ProtocolParser.h"
 #include "nsContentUtils.h"
 #include "mozilla/dom/ContentChild.h"
 #include "mozilla/dom/PermissionMessageUtils.h"
 #include "mozilla/dom/URLClassifierChild.h"
 #include "mozilla/ipc/URIUtils.h"
+#include "nsProxyRelease.h"
 
 namespace mozilla {
 namespace safebrowsing {
 
 nsresult
 TablesToResponse(const nsACString& tables)
 {
   if (tables.IsEmpty()) {
@@ -1669,16 +1670,83 @@ nsUrlClassifierDBService::ClassifyLocal(
       aTableResults.AppendLiteral(",");
     }
     aTableResults.Append(result);
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsUrlClassifierDBService::AsyncClassifyLocalWithTables(nsIURI *aURI,
+                                                       const nsACString& aTables,
+                                                       nsIURIClassifierCallback* aCallback)
+{
+  MOZ_ASSERT(NS_IsMainThread(), "AsyncClassifyLocalWithTables must be called "
+                                "on main thread");
+
+  if (XRE_IsContentProcess()) {
+    // TODO: e10s support. Bug 1343425.
+    return NS_ERROR_NOT_IMPLEMENTED;
+  }
+
+  if (gShuttingDownThread) {
+    return NS_ERROR_ABORT;
+  }
+
+  nsCOMPtr<nsIURI> uri = NS_GetInnermostURI(aURI);
+  NS_ENSURE_TRUE(uri, NS_ERROR_FAILURE);
+
+  nsAutoCString key;
+  // Canonicalize the url
+  nsCOMPtr<nsIUrlClassifierUtils> utilsService =
+    do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
+  nsresult rv = utilsService->GetKeyForURI(uri, key);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  auto worker = mWorker;
+  nsCString tables(aTables);
+
+  // Since aCallback will be passed around threads...
+  nsMainThreadPtrHandle<nsIURIClassifierCallback> callback(
+    new nsMainThreadPtrHolder<nsIURIClassifierCallback>(aCallback));
+
+  nsCOMPtr<nsIRunnable> r =
+    NS_NewRunnableFunction([worker, key, tables, callback] () -> void {
+
+    nsCString matchedLists;
+    nsAutoPtr<LookupResultArray> results(new LookupResultArray());
+    if (results) {
+      nsresult rv = worker->DoLocalLookup(key, tables, results);
+      if (NS_SUCCEEDED(rv)) {
+        for (uint32_t i = 0; i < results->Length(); i++) {
+          if (i > 0) {
+            matchedLists.AppendLiteral(",");
+          }
+          matchedLists.Append(results->ElementAt(i).mTableName);
+        }
+      }
+    }
+
+    nsCOMPtr<nsIRunnable> cbRunnable =
+      NS_NewRunnableFunction([callback, matchedLists] () -> void {
+        // |callback| is captured as const value so ...
+        auto cb = const_cast<nsIURIClassifierCallback*>(callback.get());
+        cb->OnClassifyComplete(NS_OK,           // Not used.
+                               matchedLists,
+                               EmptyCString(),  // provider. (Not used)
+                               EmptyCString()); // prefix. (Not used)
+      });
+
+    NS_DispatchToMainThread(cbRunnable);
+  });
+
+  return gDbBackgroundThread->Dispatch(r, NS_DISPATCH_NORMAL);
+}
+
+NS_IMETHODIMP
 nsUrlClassifierDBService::ClassifyLocalWithTables(nsIURI *aURI,
                                                   const nsACString& aTables,
                                                   nsTArray<nsCString>& aTableResults)
 {
   MOZ_ASSERT(NS_IsMainThread(), "ClassifyLocalWithTables must be on main thread");
   if (gShuttingDownThread) {
     return NS_ERROR_ABORT;
   }
--- a/toolkit/components/url-classifier/tests/mochitest/test_classifier.html
+++ b/toolkit/components/url-classifier/tests/mochitest/test_classifier.html
@@ -138,17 +138,32 @@ function testService() {
       let tables = "test-malware-simple,test-unwanted-simple,test-phish-simple,test-track-simple,test-block-simple";
       let uri = ios.newURI(test.url);
       let prin = ssm.createCodebasePrincipal(uri, {});
       is(service.classifyLocal(uri, tables), test.table,
          `Successful synchronous classification of ${test.url} with TP=${test.trackingProtection}`);
       let result = service.classify(prin, test.trackingProtection, function(errorCode) {
         is(errorCode, test.result,
            `Successful asynchronous classification of ${test.url} with TP=${test.trackingProtection}`);
-        runNextTest();
+
+        try {
+          // Same as classifyLocal except for the 'async' call.
+          service.asyncClassifyLocalWithTables(uri, tables, function(errorCode, tables) {
+            is(tables, test.table,
+               `Successful asynchronous local classification of ${test.url} with TP=${test.trackingProtection}`);
+            runNextTest();
+          });
+        } catch (e) {
+          // TODO: asyncClassifyLocalWithTables e10s support. See Bug 1343425.
+          let processType = Cc["@mozilla.org/xre/app-info;1"]
+                              .getService(Ci.nsIXULRuntime).processType;
+          isnot(processType, Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT,
+                "Local asynchronous classification is not supported in content process");
+          runNextTest();
+        }
       });
     }
     runNextTest(resolve);
   });
 }
 
 SpecialPowers.pushPrefEnv(
   {"set" : [["urlclassifier.malwareTable", "test-malware-simple,test-unwanted-simple"],