Bug 998396 - Fix gethash completions, urlclassifier.malware_table and urlclassifier.phish_table may be comma-separated lists. r=gcp, a=lsblakk
authorMonica Chew <mmc@mozilla.com>
Tue, 22 Apr 2014 09:46:46 -0700
changeset 227394 8fc77ee3972db19e3c3c3cdb1974ddef58e8a3ca
parent 227393 f154d04c6c08f17c314cc1e960419b1288a6fff0
child 227395 622ae811fe066895098cda6f465ae3cd133eeb30
push id6
push userryanvm@gmail.com
push dateMon, 12 Jan 2015 22:04:06 +0000
treeherdermozilla-b2g37_v2_2@895c8fc7b734 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp, lsblakk
bugs998396
milestone30.0a2
Bug 998396 - Fix gethash completions, urlclassifier.malware_table and urlclassifier.phish_table may be comma-separated lists. r=gcp, a=lsblakk
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.h
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -25,16 +25,17 @@
 #include "nsString.h"
 #include "nsReadableUtils.h"
 #include "nsTArray.h"
 #include "nsNetUtil.h"
 #include "nsNetCID.h"
 #include "nsThreadUtils.h"
 #include "nsXPCOMStrings.h"
 #include "nsProxyRelease.h"
+#include "nsString.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Telemetry.h"
 #include "prlog.h"
 #include "prprf.h"
@@ -64,16 +65,17 @@ PRLogModuleInfo *gUrlClassifierDbService
 #define CHECK_MALWARE_DEFAULT   false
 
 #define CHECK_PHISHING_PREF     "browser.safebrowsing.enabled"
 #define CHECK_PHISHING_DEFAULT  false
 
 #define GETHASH_NOISE_PREF      "urlclassifier.gethashnoise"
 #define GETHASH_NOISE_DEFAULT   4
 
+// Comma-separated lists
 #define MALWARE_TABLE_PREF      "urlclassifier.malware_table"
 #define PHISH_TABLE_PREF        "urlclassifier.phish_table"
 #define DOWNLOAD_BLOCK_TABLE_PREF "urlclassifier.downloadBlockTable"
 #define DOWNLOAD_ALLOW_TABLE_PREF "urlclassifier.downloadAllowTable"
 #define DISALLOW_COMPLETION_TABLE_PREF "urlclassifier.disallow_completions"
 
 #define CONFIRM_AGE_PREF        "urlclassifier.max-complete-age"
 #define CONFIRM_AGE_DEFAULT_SEC (45 * 60)
@@ -1066,41 +1068,66 @@ nsUrlClassifierDBService::nsUrlClassifie
 }
 
 nsUrlClassifierDBService::~nsUrlClassifierDBService()
 {
   sUrlClassifierDBService = nullptr;
 }
 
 nsresult
+nsUrlClassifierDBService::ReadTablesFromPrefs()
+{
+  nsCString allTables;
+  nsCString tables;
+  Preferences::GetCString(PHISH_TABLE_PREF, &allTables);
+
+  Preferences::GetCString(MALWARE_TABLE_PREF, &tables);
+  if (!tables.IsEmpty()) {
+    allTables.Append(',');
+    allTables.Append(tables);
+  }
+
+  Preferences::GetCString(DOWNLOAD_BLOCK_TABLE_PREF, &tables);
+  if (!tables.IsEmpty()) {
+    allTables.Append(',');
+    allTables.Append(tables);
+  }
+
+  Preferences::GetCString(DOWNLOAD_ALLOW_TABLE_PREF, &tables);
+  if (!tables.IsEmpty()) {
+    allTables.Append(',');
+    allTables.Append(tables);
+  }
+
+  Classifier::SplitTables(allTables, mGethashTables);
+
+  Preferences::GetCString(DISALLOW_COMPLETION_TABLE_PREF, &tables);
+  Classifier::SplitTables(tables, mDisallowCompletionsTables);
+
+  return NS_OK;
+}
+
+nsresult
 nsUrlClassifierDBService::Init()
 {
 #if defined(PR_LOGGING)
   if (!gUrlClassifierDbServiceLog)
     gUrlClassifierDbServiceLog = PR_NewLogModule("UrlClassifierDbService");
 #endif
 
   // Retrieve all the preferences.
   mCheckMalware = Preferences::GetBool(CHECK_MALWARE_PREF,
     CHECK_MALWARE_DEFAULT);
   mCheckPhishing = Preferences::GetBool(CHECK_PHISHING_PREF,
     CHECK_PHISHING_DEFAULT);
   uint32_t gethashNoise = Preferences::GetUint(GETHASH_NOISE_PREF,
     GETHASH_NOISE_DEFAULT);
   gFreshnessGuarantee = Preferences::GetInt(CONFIRM_AGE_PREF,
     CONFIRM_AGE_DEFAULT_SEC);
-  mGethashTables.AppendElement(Preferences::GetCString(PHISH_TABLE_PREF));
-  mGethashTables.AppendElement(Preferences::GetCString(MALWARE_TABLE_PREF));
-  mGethashTables.AppendElement(Preferences::GetCString(
-    DOWNLOAD_BLOCK_TABLE_PREF));
-  mGethashTables.AppendElement(Preferences::GetCString(
-    DOWNLOAD_ALLOW_TABLE_PREF));
-  nsCString tables;
-  Preferences::GetCString(DISALLOW_COMPLETION_TABLE_PREF, &tables);
-  Classifier::SplitTables(tables, mDisallowCompletionsTables);
+  ReadTablesFromPrefs();
 
   // Do we *really* need to be able to change all of these at runtime?
   Preferences::AddStrongObserver(this, CHECK_MALWARE_PREF);
   Preferences::AddStrongObserver(this, CHECK_PHISHING_PREF);
   Preferences::AddStrongObserver(this, GETHASH_NOISE_PREF);
   Preferences::AddStrongObserver(this, CONFIRM_AGE_PREF);
   Preferences::AddStrongObserver(this, PHISH_TABLE_PREF);
   Preferences::AddStrongObserver(this, MALWARE_TABLE_PREF);
@@ -1167,16 +1194,17 @@ nsUrlClassifierDBService::Classify(nsIPr
   }
 
   nsRefPtr<nsUrlClassifierClassifyCallback> callback =
     new nsUrlClassifierClassifyCallback(c, mCheckMalware, mCheckPhishing);
   if (!callback) return NS_ERROR_OUT_OF_MEMORY;
 
   nsAutoCString tables;
   nsAutoCString malware;
+  // LookupURI takes a comma-separated list already.
   Preferences::GetCString(MALWARE_TABLE_PREF, &malware);
   if (!malware.IsEmpty()) {
     tables.Append(malware);
   }
   nsAutoCString phishing;
   Preferences::GetCString(PHISH_TABLE_PREF, &phishing);
   if (!phishing.IsEmpty()) {
     tables.Append(",");
@@ -1405,16 +1433,19 @@ nsUrlClassifierDBService::GetCompleter(c
 
   // If we don't know about this table at all, or are disallowing completions
   // for it, skip completion checks.
   if (!mGethashTables.Contains(tableName) ||
       mDisallowCompletionsTables.Contains(tableName)) {
     return false;
   }
 
+  MOZ_ASSERT(!StringBeginsWith(tableName, NS_LITERAL_CSTRING("test-")),
+             "We should never fetch hash completions for test tables");
+
   // Otherwise, call gethash to find the hash completions.
   return NS_SUCCEEDED(CallGetService(NS_URLCLASSIFIERHASHCOMPLETER_CONTRACTID,
                                      completer));
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBService::Observe(nsISupports *aSubject, const char *aTopic,
                                   const char16_t *aData)
@@ -1424,33 +1455,24 @@ nsUrlClassifierDBService::Observe(nsISup
     nsCOMPtr<nsIPrefBranch> prefs(do_QueryInterface(aSubject, &rv));
     NS_ENSURE_SUCCESS(rv, rv);
     if (NS_LITERAL_STRING(CHECK_MALWARE_PREF).Equals(aData)) {
       mCheckMalware = Preferences::GetBool(CHECK_MALWARE_PREF,
         CHECK_MALWARE_DEFAULT);
     } else if (NS_LITERAL_STRING(CHECK_PHISHING_PREF).Equals(aData)) {
       mCheckPhishing = Preferences::GetBool(CHECK_PHISHING_PREF,
         CHECK_PHISHING_DEFAULT);
-    } else if (NS_LITERAL_STRING(PHISH_TABLE_PREF).Equals(aData) ||
-               NS_LITERAL_STRING(MALWARE_TABLE_PREF).Equals(aData) ||
-               NS_LITERAL_STRING(DOWNLOAD_BLOCK_TABLE_PREF).Equals(aData) ||
-               NS_LITERAL_STRING(DOWNLOAD_ALLOW_TABLE_PREF).Equals(aData)) {
+    } else if (
+      NS_LITERAL_STRING(PHISH_TABLE_PREF).Equals(aData) ||
+      NS_LITERAL_STRING(MALWARE_TABLE_PREF).Equals(aData) ||
+      NS_LITERAL_STRING(DOWNLOAD_BLOCK_TABLE_PREF).Equals(aData) ||
+      NS_LITERAL_STRING(DOWNLOAD_ALLOW_TABLE_PREF).Equals(aData) ||
+      NS_LITERAL_STRING(DISALLOW_COMPLETION_TABLE_PREF).Equals(aData)) {
       // Just read everything again.
-      mGethashTables.Clear();
-      mGethashTables.AppendElement(Preferences::GetCString(PHISH_TABLE_PREF));
-      mGethashTables.AppendElement(Preferences::GetCString(MALWARE_TABLE_PREF));
-      mGethashTables.AppendElement(Preferences::GetCString(
-        DOWNLOAD_BLOCK_TABLE_PREF));
-      mGethashTables.AppendElement(Preferences::GetCString(
-        DOWNLOAD_ALLOW_TABLE_PREF));
-    } else if (NS_LITERAL_STRING(DISALLOW_COMPLETION_TABLE_PREF).Equals(aData)) {
-      mDisallowCompletionsTables.Clear();
-      nsCString tables;
-      Preferences::GetCString(DISALLOW_COMPLETION_TABLE_PREF, &tables);
-      Classifier::SplitTables(tables, mDisallowCompletionsTables);
+      ReadTablesFromPrefs();
     } else if (NS_LITERAL_STRING(CONFIRM_AGE_PREF).Equals(aData)) {
       gFreshnessGuarantee = Preferences::GetInt(CONFIRM_AGE_PREF,
         CONFIRM_AGE_DEFAULT_SEC);
     }
   } else if (!strcmp(aTopic, "profile-before-change") ||
              !strcmp(aTopic, "xpcom-shutdown-threads")) {
     Shutdown();
   } else {
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -77,16 +77,19 @@ private:
 
   // Close db connection and join the background thread if it exists.
   nsresult Shutdown();
 
   // Check if the key is on a known-clean host.
   nsresult CheckClean(const nsACString &lookupKey,
                       bool *clean);
 
+  // Read everything into mGethashTables and mDisallowCompletionTables
+  nsresult ReadTablesFromPrefs();
+
   nsCOMPtr<nsUrlClassifierDBServiceWorker> mWorker;
   nsCOMPtr<nsIUrlClassifierDBServiceWorker> mWorkerProxy;
 
   nsInterfaceHashtable<nsCStringHashKey, nsIUrlClassifierHashCompleter> mCompleters;
 
   // TRUE if the nsURIClassifier implementation should check for malware
   // uris on document loads.
   bool mCheckMalware;