Bug 1319571 - Change the output format of nsUrlClassifierDBService::ClassifyLocalWithTables from comma separated string to an array of strings draft
authorKirk Steuber <ksteuber@mozilla.com>
Tue, 29 Nov 2016 13:04:43 -0800
changeset 445554 8b8ce8435f1cc8216f107c915b63138c2cd762d3
parent 444725 8387a4ada9a5c4cab059d8fafe0f8c933e83c149
child 538559 56103a082cb579e7d1bda9edf7450472c6e12f17
push id37552
push userksteuber@mozilla.com
push dateTue, 29 Nov 2016 23:27:58 +0000
bugs1319571
milestone53.0a1
Bug 1319571 - Change the output format of nsUrlClassifierDBService::ClassifyLocalWithTables from comma separated string to an array of strings MozReview-Commit-ID: TXln2EQnZS
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
dom/ipc/PContent.ipdl
netwerk/base/nsChannelClassifier.cpp
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/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -4806,17 +4806,17 @@ ContentParent::DeallocPURLClassifierPare
 
   RefPtr<URLClassifierParent> actor =
     dont_AddRef(static_cast<URLClassifierParent*>(aActor));
   return true;
 }
 
 mozilla::ipc::IPCResult
 ContentParent::RecvClassifyLocal(const URIParams& aURI, const nsCString& aTables,
-                                 nsCString* aResults)
+                                 nsTArray<nsCString>* aResults)
 {
   MOZ_ASSERT(aResults);
   nsCOMPtr<nsIURI> uri = DeserializeURI(aURI);
   if (!uri) {
     return IPC_FAIL_NO_REASON(this);
   }
   nsCOMPtr<nsIURIClassifier> uriClassifier =
     do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID);
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -555,17 +555,17 @@ public:
                                 const bool& aUseTrackingProtection,
                                 bool* aSuccess) override;
   virtual bool
   DeallocPURLClassifierParent(PURLClassifierParent* aActor) override;
 
   virtual mozilla::ipc::IPCResult
   RecvClassifyLocal(const URIParams& aURI,
                     const nsCString& aTables,
-                    nsCString* aResults) override;
+                    nsTArray<nsCString>* aResults) override;
 
   // Use the PHangMonitor channel to ask the child to repaint a tab.
   void ForceTabPaint(TabParent* aTabParent, uint64_t aLayerObserverEpoch);
 
 protected:
   void OnChannelConnected(int32_t pid) override;
 
   virtual void ActorDestroy(ActorDestroyReason why) override;
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -799,17 +799,17 @@ parent:
 
     async PPresentation();
 
     async PFlyWebPublishedServer(nsString name, FlyWebPublishOptions params);
 
     sync PURLClassifier(Principal principal, bool useTrackingProtection)
         returns (bool success);
     sync ClassifyLocal(URIParams uri, nsCString tables)
-        returns (nsCString results);
+        returns (nsCString[] results);
 
     // Services remoting
 
     async StartVisitedQuery(URIParams uri);
     async VisitURI(URIParams uri, OptionalURIParams referrer, uint32_t flags);
     async SetURITitle(URIParams uri, nsString title);
 
     async LoadURIExternal(URIParams uri, PBrowser windowContext);
--- a/netwerk/base/nsChannelClassifier.cpp
+++ b/netwerk/base/nsChannelClassifier.cpp
@@ -628,17 +628,17 @@ nsChannelClassifier::IsTrackerWhiteliste
   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
-  nsAutoCString results;
+  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
   }
 
   LOG(("nsChannelClassifier[%p]: %s is not in the whitelist",
        this, whitelistEntry.get()));
--- a/netwerk/base/nsIURIClassifier.idl
+++ b/netwerk/base/nsIURIClassifier.idl
@@ -1,14 +1,20 @@
 /* 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"
 
+%{C++
+#include "nsTArrayForwardDeclare.h"
+class nsCString;
+%}
+[ref] native StringArrayRef(nsTArray<nsCString>);
+
 interface nsIChannel;
 interface nsIPrincipal;
 interface nsIURI;
 
 /**
  * Callback function for nsIURIClassifier lookups.
  */
 [scriptable, function, uuid(8face46e-0c96-470f-af40-0037dcd797bd)]
@@ -54,12 +60,17 @@ interface nsIURIClassifier : nsISupports
    */
   boolean classify(in nsIPrincipal aPrincipal,
                    in boolean aTrackingProtectionEnabled,
                    in nsIURIClassifierCallback aCallback);
 
   /**
    * Synchronously classify a URI with a comma-separated string
    * containing the given tables. This does not make network requests.
-   * The result is a comma-separated string of tables that match.
+   * The result is an array of table names that match.
    */
-  ACString classifyLocalWithTables(in nsIURI aURI, in ACString aTables);
+  [noscript] StringArrayRef classifyLocalWithTables(in nsIURI aURI, in ACString aTables);
+  /**
+   * 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
@@ -5896,17 +5896,17 @@ nsHttpChannel::BeginConnect()
             // 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;
             rv = GetURI(getter_AddRefs(uri));
             if (NS_SUCCEEDED(rv) && uri) {
                 nsAutoCString tables;
                 Preferences::GetCString("urlclassifier.trackingTable", &tables);
-                nsAutoCString results;
+                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 "
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -209,36 +209,29 @@ nsUrlClassifierDBServiceWorker::DoLocalL
   // We ignore failures from Check because we'd rather return the
   // results that were found than fail.
   mClassifier->Check(spec, tables, gFreshnessGuarantee, *results);
 
   LOG(("Found %d results.", results->Length()));
   return NS_OK;
 }
 
-static nsCString
-ProcessLookupResults(LookupResultArray* results)
+static nsresult
+ProcessLookupResults(LookupResultArray* results, nsTArray<nsCString>& tables)
 {
-  // Build a stringified list of result tables.
-  nsTArray<nsCString> tables;
+  // Build the result array.
   for (uint32_t i = 0; i < results->Length(); i++) {
     LookupResult& result = results->ElementAt(i);
     MOZ_ASSERT(!result.mNoise, "Lookup results should not have noise added");
     LOG(("Found result from table %s", result.mTableName.get()));
     if (tables.IndexOf(result.mTableName) == nsTArray<nsCString>::NoIndex) {
       tables.AppendElement(result.mTableName);
     }
   }
-  nsAutoCString tableStr;
-  for (uint32_t i = 0; i < tables.Length(); i++) {
-    if (i != 0)
-      tableStr.Append(',');
-    tableStr.Append(tables[i]);
-  }
-  return tableStr;
+  return NS_OK;
 }
 
 /**
  * Lookup up a key in the database is a two step process:
  *
  * a) First we look for any Entries in the database that might apply to this
  *    url.  For each URL there are one or two possible domain names to check:
  *    the two-part domain name (example.com) and the three-part name
@@ -1488,36 +1481,56 @@ nsUrlClassifierDBService::Classify(nsIPr
     return NS_OK;
   }
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsUrlClassifierDBService::ClassifyLocal(nsIURI *aURI,
+                                        const nsACString& aTables,
+                                        nsACString& aTableResults)
+{
+  nsTArray<nsCString> results;
+  ClassifyLocalWithTables(aURI, aTables, results);
+
+  // Convert the result array to a comma separated string
+  aTableResults.AssignLiteral("");
+  bool first = true;
+  for (nsCString& result : results) {
+    if (first) {
+      first = false;
+    } else {
+      aTableResults.AppendLiteral(",");
+    }
+    aTableResults.Append(result);
+  }
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 nsUrlClassifierDBService::ClassifyLocalWithTables(nsIURI *aURI,
-                                                  const nsACString & aTables,
-                                                  nsACString & aTableResults)
+                                                  const nsACString& aTables,
+                                                  nsTArray<nsCString>& aTableResults)
 {
   MOZ_ASSERT(NS_IsMainThread(), "ClassifyLocalWithTables must be on main thread");
   if (gShuttingDownThread) {
     return NS_ERROR_ABORT;
   }
 
   if (XRE_IsContentProcess()) {
     using namespace mozilla::dom;
     using namespace mozilla::ipc;
     URIParams uri;
     SerializeURI(aURI, uri);
     nsAutoCString tables(aTables);
-    nsAutoCString results;
     bool result = ContentChild::GetSingleton()->SendClassifyLocal(uri, tables,
-                                                                  &results);
+                                                                  &aTableResults);
     if (result) {
-      aTableResults = results;
       return NS_OK;
     }
     return NS_ERROR_FAILURE;
   }
 
   PROFILER_LABEL_FUNC(js::ProfileEntry::Category::OTHER);
 
   nsCOMPtr<nsIURI> uri = NS_GetInnermostURI(aURI);
@@ -1533,17 +1546,18 @@ nsUrlClassifierDBService::ClassifyLocalW
   nsAutoPtr<LookupResultArray> results(new LookupResultArray());
   if (!results) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   // In unittests, we may not have been initalized, so don't crash.
   rv = mWorkerProxy->DoLocalLookup(key, aTables, results);
   if (NS_SUCCEEDED(rv)) {
-    aTableResults = ProcessLookupResults(results);
+    rv = ProcessLookupResults(results, aTableResults);
+    NS_ENSURE_SUCCESS(rv, rv);
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBService::Lookup(nsIPrincipal* aPrincipal,
                                  const nsACString& tables,
                                  nsIUrlClassifierCallback* c)
--- a/toolkit/components/url-classifier/tests/mochitest/test_classifier.html
+++ b/toolkit/components/url-classifier/tests/mochitest/test_classifier.html
@@ -133,17 +133,17 @@ function testService() {
       if (!testURLs.length) {
         resolve();
         return;
       }
       let test = testURLs.shift();
       let tables = "test-malware-simple,test-unwanted-simple,test-phish-simple,test-track-simple,test-block-simple";
       let uri = ios.newURI(test.url, null, null);
       let prin = ssm.createCodebasePrincipal(uri, {});
-      is(service.classifyLocalWithTables(uri, tables), test.table,
+      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();
       });
     }
     runNextTest(resolve);