Bug 778606 - SafeBrowsing.jsm should use nsUrlFormatter. r=gcp, f=gavin
authorJustin Dolske <dolske@mozilla.com>
Wed, 01 Aug 2012 15:52:47 -0700
changeset 101164 a799b5bff84c841a6d35eb5d079ba308a122f675
parent 101163 7960368f7a22df60a69d10b0c81aa148ba796bed
child 101165 f0ac9e6acaa5beff29e02c789468b172a15e510b
push id12875
push userjdolske@mozilla.com
push dateWed, 01 Aug 2012 22:52:32 +0000
treeherdermozilla-inbound@a799b5bff84c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs778606
milestone17.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 778606 - SafeBrowsing.jsm should use nsUrlFormatter. r=gcp, f=gavin
browser/app/profile/firefox.js
browser/components/safebrowsing/Makefile.in
browser/components/safebrowsing/SafeBrowsing.jsm
build/automation.py.in
toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -710,46 +710,32 @@ pref("gecko.handlerService.schemes.ircs.
 pref("gecko.handlerService.schemes.ircs.2.uriTemplate", "chrome://browser-region/locale/region.properties");
 pref("gecko.handlerService.schemes.ircs.3.name", "chrome://browser-region/locale/region.properties");
 pref("gecko.handlerService.schemes.ircs.3.uriTemplate", "chrome://browser-region/locale/region.properties");
 
 // By default, we don't want protocol/content handlers to be registered from a different host, see bug 402287
 pref("gecko.handlerService.allowRegisterFromDifferentHost", false);
 
 #ifdef MOZ_SAFE_BROWSING
-// Safe browsing does nothing unless this pref is set
 pref("browser.safebrowsing.enabled", true);
-
-// Prevent loading of pages identified as malware
 pref("browser.safebrowsing.malware.enabled", true);
-
-// Debug logging to error console
 pref("browser.safebrowsing.debug", false);
 
-// Non-enhanced mode (local url lists) URL list to check for updates
-pref("browser.safebrowsing.provider.0.updateURL", "http://safebrowsing.clients.google.com/safebrowsing/downloads?client={moz:client}&appver={moz:version}&pver=2.2");
-
-pref("browser.safebrowsing.dataProvider", 0);
-
-// Does the provider name need to be localizable?
-pref("browser.safebrowsing.provider.0.name", "Google");
-pref("browser.safebrowsing.provider.0.keyURL", "https://sb-ssl.google.com/safebrowsing/newkey?client={moz:client}&appver={moz:version}&pver=2.2");
-pref("browser.safebrowsing.provider.0.reportURL", "http://safebrowsing.clients.google.com/safebrowsing/report?");
-pref("browser.safebrowsing.provider.0.gethashURL", "http://safebrowsing.clients.google.com/safebrowsing/gethash?client={moz:client}&appver={moz:version}&pver=2.2");
+pref("browser.safebrowsing.updateURL", "http://safebrowsing.clients.google.com/safebrowsing/downloads?client=SAFEBROWSING_ID&appver=%VERSION%&pver=2.2");
+pref("browser.safebrowsing.keyURL", "https://sb-ssl.google.com/safebrowsing/newkey?client=SAFEBROWSING_ID&appver=%VERSION%&pver=2.2");
+pref("browser.safebrowsing.gethashURL", "http://safebrowsing.clients.google.com/safebrowsing/gethash?client=SAFEBROWSING_ID&appver=%VERSION%&pver=2.2");
+pref("browser.safebrowsing.reportURL", "http://safebrowsing.clients.google.com/safebrowsing/report?");
+pref("browser.safebrowsing.reportGenericURL", "http://%LOCALE%.phish-generic.mozilla.com/?hl=%LOCALE%");
+pref("browser.safebrowsing.reportErrorURL", "http://%LOCALE%.phish-error.mozilla.com/?hl=%LOCALE%");
+pref("browser.safebrowsing.reportPhishURL", "http://%LOCALE%.phish-report.mozilla.com/?hl=%LOCALE%");
+pref("browser.safebrowsing.reportMalwareURL", "http://%LOCALE%.malware-report.mozilla.com/?hl=%LOCALE%");
+pref("browser.safebrowsing.reportMalwareErrorURL", "http://%LOCALE%.malware-error.mozilla.com/?hl=%LOCALE%");
 
-// HTML report pages
-pref("browser.safebrowsing.provider.0.reportGenericURL", "http://{moz:locale}.phish-generic.mozilla.com/?hl={moz:locale}");
-pref("browser.safebrowsing.provider.0.reportErrorURL", "http://{moz:locale}.phish-error.mozilla.com/?hl={moz:locale}");
-pref("browser.safebrowsing.provider.0.reportPhishURL", "http://{moz:locale}.phish-report.mozilla.com/?hl={moz:locale}");
-pref("browser.safebrowsing.provider.0.reportMalwareURL", "http://{moz:locale}.malware-report.mozilla.com/?hl={moz:locale}");
-pref("browser.safebrowsing.provider.0.reportMalwareErrorURL", "http://{moz:locale}.malware-error.mozilla.com/?hl={moz:locale}");
-
-// FAQ URLs
 pref("browser.safebrowsing.warning.infoURL", "http://www.mozilla.com/%LOCALE%/firefox/phishing-protection/");
-pref("browser.geolocation.warning.infoURL", "http://www.mozilla.com/%LOCALE%/firefox/geolocation/");
+pref("browser.safebrowsing.malware.reportURL", "http://safebrowsing.clients.google.com/safebrowsing/diagnostic?client=%NAME%&hl=%LOCALE%&site=");
 
 // Name of the about: page contributed by safebrowsing to handle display of error
 // pages on phishing/malware hits.  (bug 399233)
 pref("urlclassifier.alternate_error_page", "blocked");
 
 // The number of random entries to send with a gethash request.
 pref("urlclassifier.gethashnoise", 4);
 
@@ -761,21 +747,19 @@ pref("urlclassifier.gethashtables", "goo
 // the database.
 pref("urlclassifier.confirm-age", 2700);
 
 // Maximum size of the sqlite3 cache during an update, in bytes
 pref("urlclassifier.updatecachemax", 41943040);
 
 // Maximum size of the sqlite3 cache for lookups, in bytes
 pref("urlclassifier.lookupcachemax", 1048576);
+#endif
 
-// URL for checking the reason for a malware warning.
-pref("browser.safebrowsing.malware.reportURL", "http://safebrowsing.clients.google.com/safebrowsing/diagnostic?client=%NAME%&hl=%LOCALE%&site=");
-
-#endif
+pref("browser.geolocation.warning.infoURL", "http://www.mozilla.com/%LOCALE%/firefox/geolocation/");
 
 pref("browser.EULA.version", 3);
 pref("browser.rights.version", 3);
 pref("browser.rights.3.shown", false);
 
 #ifdef DEBUG
 // Don't show the about:rights notification in debug builds.
 pref("browser.rights.override", true);
--- a/browser/components/safebrowsing/Makefile.in
+++ b/browser/components/safebrowsing/Makefile.in
@@ -1,24 +1,27 @@
-#
 # 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/.
 
 
 DEPTH     = ../../..
 topsrcdir = @top_srcdir@
 srcdir    = @srcdir@
 VPATH     = @srcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
 TEST_DIRS += content/test
 
+# Normally the "client ID" sent in updates is appinfo.name, but for
+# official Firefox releases from Mozilla we use a special identifier.
 ifdef MOZILLA_OFFICIAL
-DEFINES += -DOFFICIAL_BUILD=1
+ifdef MOZ_PHOENIX
+DEFINES += -DUSE_HISTORIC_SAFEBROWSING_ID=1
+endif
 endif
 
 EXTRA_PP_JS_MODULES = \
   SafeBrowsing.jsm \
   $(NULL)
 
 include $(topsrcdir)/config/rules.mk
--- a/browser/components/safebrowsing/SafeBrowsing.jsm
+++ b/browser/components/safebrowsing/SafeBrowsing.jsm
@@ -29,50 +29,39 @@ var SafeBrowsing = {
     if (this.initialized) {
       log("Already initialized");
       return;
     }
 
     Services.prefs.addObserver("browser.safebrowsing", this.readPrefs, false);
     this.readPrefs();
 
-    this.initProviderURLs();
-
-    // Initialize the list manager
+    // Register our two types of tables, and add custom Mozilla entries
     let listManager = Cc["@mozilla.org/url-classifier/listmanager;1"].
                       getService(Ci.nsIUrlListManager);
-
-    listManager.setUpdateUrl(this.updateURL);
-    // setKeyUrl has the side effect of fetching a key from the server.
-    // This shouldn't happen if anti-phishing/anti-malware is disabled.
-    if (this.phishingEnabled || this.malwareEnabled)
-      listManager.setKeyUrl(this.keyURL);
-    listManager.setGethashUrl(this.gethashURL);
-
-    // Register our two types of tables
     listManager.registerTable(phishingList, false);
     listManager.registerTable(malwareList, false);
     this.addMozEntries();
 
     this.controlUpdateChecking();
     this.initialized = true;
 
     log("init() finished");
   },
 
 
   initialized:     false,
   phishingEnabled: false,
   malwareEnabled:  false,
 
-  provName:              null,
   updateURL:             null,
   keyURL:                null,
+  gethashURL:            null,
+
   reportURL:             null,
-  gethashURL:            null,
   reportGenericURL:      null,
   reportErrorURL:        null,
   reportPhishURL:        null,
   reportMalwareURL:      null,
   reportMalwareErrorURL: null,
 
 
   getReportURL: function(kind) {
@@ -81,86 +70,62 @@ var SafeBrowsing = {
 
 
   readPrefs: function() {
     log("reading prefs");
 
     debug = Services.prefs.getBoolPref("browser.safebrowsing.debug");
     this.phishingEnabled = Services.prefs.getBoolPref("browser.safebrowsing.enabled");
     this.malwareEnabled  = Services.prefs.getBoolPref("browser.safebrowsing.malware.enabled");
+    this.updateProviderURLs();
 
     // XXX The listManager backend gets confused if this is called before the
     // lists are registered. So only call it here when a pref changes, and not
     // when doing initialization. I expect to refactor this later, so pardon the hack.
     if (this.initialized)
       this.controlUpdateChecking();
   },
 
 
-  initProviderURLs: function() {
-    log("initializing provider URLs");
+  updateProviderURLs: function() {
+#ifdef USE_HISTORIC_SAFEBROWSING_ID
+    let clientID = "navclient-auto-ffox";
+#else
+    let clientID = Services.appinfo.name;
+#endif
 
-    // XXX remove this as obsolete?
-    let provID = Services.prefs.getIntPref("browser.safebrowsing.dataProvider");
-    if (provID != 0) {
-      Cu.reportError("unknown safebrowsing provider ID " + provID);
-      return;
-    }
-
-    let basePref = "browser.safebrowsing.provider.0.";
-    this.provName = Services.prefs.getCharPref(basePref + "name");
-
-    // Urls used to get data from a provider
-    this.updateURL  = this.getUrlPref(basePref + "updateURL");
-    this.keyURL     = this.getUrlPref(basePref + "keyURL");
-    this.reportURL  = this.getUrlPref(basePref + "reportURL");
-    this.gethashURL = this.getUrlPref(basePref + "gethashURL");
+    log("initializing safe browsing URLs");
+    let basePref = "browser.safebrowsing.";
 
     // Urls to HTML report pages
-    this.reportGenericURL      = this.getUrlPref(basePref + "reportGenericURL");
-    this.reportErrorURL        = this.getUrlPref(basePref + "reportErrorURL");
-    this.reportPhishURL        = this.getUrlPref(basePref + "reportPhishURL");
-    this.reportMalwareURL      = this.getUrlPref(basePref + "reportMalwareURL")
-    this.reportMalwareErrorURL = this.getUrlPref(basePref + "reportMalwareErrorURL")
-  },
-
+    this.reportURL             = Services.urlFormatter.formatURLPref(basePref + "reportURL");
+    this.reportGenericURL      = Services.urlFormatter.formatURLPref(basePref + "reportGenericURL");
+    this.reportErrorURL        = Services.urlFormatter.formatURLPref(basePref + "reportErrorURL");
+    this.reportPhishURL        = Services.urlFormatter.formatURLPref(basePref + "reportPhishURL");
+    this.reportMalwareURL      = Services.urlFormatter.formatURLPref(basePref + "reportMalwareURL");
+    this.reportMalwareErrorURL = Services.urlFormatter.formatURLPref(basePref + "reportMalwareErrorURL");
 
-  getUrlPref: function(prefName) {
-    let MOZ_OFFICIAL_BUILD = false;
-#ifdef OFFICIAL_BUILD
-    MOZ_OFFICIAL_BUILD = true;
-#endif
-
-    let url = Services.prefs.getCharPref(prefName);
-
-    let clientName = MOZ_OFFICIAL_BUILD ? "navclient-auto-ffox" : Services.appinfo.name;
-    let clientVersion = Services.appinfo.version;
+    // Urls used to update DB
+    this.updateURL  = Services.urlFormatter.formatURLPref(basePref + "updateURL");
+    this.keyURL     = Services.urlFormatter.formatURLPref(basePref + "keyURL");
+    this.gethashURL = Services.urlFormatter.formatURLPref(basePref + "gethashURL");
 
-    // Parameter substitution
-    // XXX: we should instead use nsIURLFormatter here.
-    url = url.replace(/\{moz:locale\}/g,  this.getLocale());
-    url = url.replace(/\{moz:client\}/g,  clientName);
-    url = url.replace(/\{moz:buildid\}/g, Services.appinfo.appBuildID);
-    url = url.replace(/\{moz:version\}/g, clientVersion);
+    this.updateURL  = this.updateURL.replace("SAFEBROWSING_ID", clientID);
+    this.keyURL     = this.keyURL.replace("SAFEBROWSING_ID", clientID);
+    this.gethashURL = this.gethashURL.replace("SAFEBROWSING_ID", clientID);
 
-    log(prefName, "is", url);
-    return url;
-  },
+    let listManager = Cc["@mozilla.org/url-classifier/listmanager;1"].
+                      getService(Ci.nsIUrlListManager);
 
-
-  getLocale: function() {
-    const localePref = "general.useragent.locale";
-
-    let locale = Services.prefs.getCharPref(localePref);
-    try {
-      // Dumb. This API only works if pref is localized or has a user value.
-      locale = Services.prefs.getComplexValue(localePref, Ci.nsIPrefLocalizedString).data;
-    } catch (e) { }
-
-    return locale;
+    listManager.setUpdateUrl(this.updateURL);
+    // XXX Bug 779317 - setKeyUrl has the side effect of fetching a key from the server.
+    // This shouldn't happen if anti-phishing/anti-malware is disabled.
+    if (this.phishingEnabled || this.malwareEnabled)
+      listManager.setKeyUrl(this.keyURL);
+    listManager.setGethashUrl(this.gethashURL);
   },
 
 
   controlUpdateChecking: function() {
     log("phishingEnabled:", this.phishingEnabled, "malwareEnabled:", this.malwareEnabled);
 
     let listManager = Cc["@mozilla.org/url-classifier/listmanager;1"].
                       getService(Ci.nsIUrlListManager);
@@ -174,17 +139,17 @@ var SafeBrowsing = {
       listManager.enableUpdate(malwareList);
     else
       listManager.disableUpdate(malwareList);
   },
 
 
   addMozEntries: function() {
     // Add test entries to the DB.
-    // XXX this should really just be done by DB itself for all moz apps.
+    // XXX bug 779008 - this could be done by DB itself?
     const phishURL   = "mozilla.org/firefox/its-a-trap.html";
     const malwareURL = "mozilla.org/firefox/its-an-attack.html";
 
     let update = "n:1000\ni:test-malware-simple\nad:1\n" +
                  "a:1:32:" + malwareURL.length + "\n" +
                  malwareURL;
     update += "n:1000\ni:test-phish-simple\nad:1\n" +
               "a:1:32:" + phishURL.length + "\n" +
--- a/build/automation.py.in
+++ b/build/automation.py.in
@@ -412,19 +412,19 @@ user_pref("geo.wifi.uri", "http://%(serv
 user_pref("geo.wifi.testing", true);
 user_pref("geo.ignore.location_filter", true);
 
 user_pref("camino.warn_when_closing", false); // Camino-only, harmless to others
 
 // Make url-classifier updates so rare that they won't affect tests
 user_pref("urlclassifier.updateinterval", 172800);
 // Point the url-classifier to the local testing server for fast failures
-user_pref("browser.safebrowsing.provider.0.gethashURL", "http://%(server)s/safebrowsing-dummy/gethash");
-user_pref("browser.safebrowsing.provider.0.keyURL", "http://%(server)s/safebrowsing-dummy/newkey");
-user_pref("browser.safebrowsing.provider.0.updateURL", "http://%(server)s/safebrowsing-dummy/update");
+user_pref("browser.safebrowsing.gethashURL", "http://%(server)s/safebrowsing-dummy/gethash");
+user_pref("browser.safebrowsing.keyURL", "http://%(server)s/safebrowsing-dummy/newkey");
+user_pref("browser.safebrowsing.updateURL", "http://%(server)s/safebrowsing-dummy/update");
 // Point update checks to the local testing server for fast failures
 user_pref("extensions.update.url", "http://%(server)s/extensions-dummy/updateURL");
 user_pref("extensions.update.background.url", "http://%(server)s/extensions-dummy/updateBackgroundURL");
 user_pref("extensions.blocklist.url", "http://%(server)s/extensions-dummy/blocklistURL");
 user_pref("extensions.hotfix.url", "http://%(server)s/extensions-dummy/hotfixURL");
 // Make sure opening about:addons won't hit the network
 user_pref("extensions.webservice.discoverURL", "http://%(server)s/extensions-dummy/discoveryURL");
 // Make sure AddonRepository won't hit the network
--- a/toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js
+++ b/toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js
@@ -221,17 +221,17 @@ HashCompleter.prototype = {
 };
 
 function HashCompleterRequest(aCompleter) {
   // HashCompleter object that created this HashCompleterRequest.
   this._completer = aCompleter;
   // The internal set of hashes and callbacks that this request corresponds to.
   this._requests = [];
   // URI to query for hash completions. Largely comes from the
-  // browser.safebrowsing.provider.#.gethashURL pref.
+  // browser.safebrowsing.gethashURL pref.
   this._uri = null;
   // nsIChannel that the hash completion query is transmitted over.
   this._channel = null;
   // Response body of hash completion. Created in onDataAvailable.
   this._response = "";
   // Client key when HMAC is used.
   this._clientKey = "";
   // Request was rescheduled, possibly due to a "e:pleaserekey" request from