Bug 1273587 - Change the User-Agent header sent to Sync to include OS information. r=kitcambridge
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 31 May 2016 09:25:10 +1000
changeset 341466 819915e031bef19e03285a576e3efb98bc829393
parent 341465 6028e2e8233925b4ae440eb68ece9f5015e5c48b
child 341467 1330c510901a9c6b574532af366f0e9acb8beb8c
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskitcambridge
bugs1273587
milestone49.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 1273587 - Change the User-Agent header sent to Sync to include OS information. r=kitcambridge The custom UA string sent by Sync now includes the "oscpu" portion of the default UA string. Eg, with this patch my UA string looks like "Firefox/49.0a1 (Windows NT 6.1; WOW64) FxSync/1.51.0.20160524173017.desktop" where the value in parenthesis is added by this patch. MozReview-Commit-ID: 1gfqQoVbO6V
services/sync/modules/resource.js
services/sync/modules/rest.js
services/sync/modules/service.js
services/sync/modules/util.js
services/sync/tests/unit/test_resource_ua.js
services/sync/tests/unit/test_service_getStorageInfo.js
services/sync/tests/unit/test_syncstoragerequest.js
--- a/services/sync/modules/resource.js
+++ b/services/sync/modules/resource.js
@@ -69,30 +69,16 @@ AsyncResource.prototype = {
   /**
    * Callback to be invoked at request time to add authentication details.
    *
    * By default, a global authenticator is provided. If this is set, it will
    * be used instead of the global one.
    */
   authenticator: null,
 
-  // The string to use as the base User-Agent in Sync requests.
-  // These strings will look something like
-  //
-  //   Firefox/4.0 FxSync/1.8.0.20100101.mobile
-  //
-  // or
-  //
-  //   Firefox Aurora/5.0a1 FxSync/1.9.0.20110409.desktop
-  //
-  _userAgent:
-    Services.appinfo.name + "/" + Services.appinfo.version +  // Product.
-    " FxSync/" + WEAVE_VERSION + "." +                        // Sync.
-    Services.appinfo.appBuildID + ".",                        // Build.
-
   // Wait 5 minutes before killing a request.
   ABORT_TIMEOUT: 300000,
 
   // ** {{{ AsyncResource.headers }}} **
   //
   // Headers to be included when making a request for the resource.
   // Note: Header names should be all lower case, there's no explicit
   // check for duplicates due to case!
@@ -156,18 +142,17 @@ AsyncResource.prototype = {
     channel.loadFlags |= DEFAULT_LOAD_FLAGS;
 
     // Setup a callback to handle channel notifications.
     let listener = new ChannelNotificationListener(this.headerNames);
     channel.notificationCallbacks = listener;
 
     // Compose a UA string fragment from the various available identifiers.
     if (Svc.Prefs.get("sendVersionInfo", true)) {
-      let ua = this._userAgent + Svc.Prefs.get("client.type", "desktop");
-      channel.setRequestHeader("user-agent", ua, false);
+      channel.setRequestHeader("user-agent", Utils.userAgent, false);
     }
 
     let headers = this.headers;
 
     if (this.authenticator) {
       let result = this.authenticator(this, method);
       if (result && result.headers) {
         for (let [k, v] in Iterator(result.headers)) {
--- a/services/sync/modules/rest.js
+++ b/services/sync/modules/rest.js
@@ -23,40 +23,24 @@ this.SyncStorageRequest = function SyncS
 }
 SyncStorageRequest.prototype = {
 
   __proto__: RESTRequest.prototype,
 
   _logName: "Sync.StorageRequest",
 
   /**
-   * The string to use as the base User-Agent in Sync requests.
-   * These strings will look something like
-   * 
-   *   Firefox/4.0 FxSync/1.8.0.20100101.mobile
-   * 
-   * or
-   * 
-   *   Firefox Aurora/5.0a1 FxSync/1.9.0.20110409.desktop
-   */
-  userAgent:
-    Services.appinfo.name + "/" + Services.appinfo.version +  // Product.
-    " FxSync/" + WEAVE_VERSION + "." +                        // Sync.
-    Services.appinfo.appBuildID + ".",                        // Build.
-
-  /**
    * Wait 5 minutes before killing a request.
    */
   timeout: STORAGE_REQUEST_TIMEOUT,
 
   dispatch: function dispatch(method, data, onComplete, onProgress) {
     // Compose a UA string fragment from the various available identifiers.
     if (Svc.Prefs.get("sendVersionInfo", true)) {
-      let ua = this.userAgent + Svc.Prefs.get("client.type", "desktop");
-      this.setHeader("user-agent", ua);
+      this.setHeader("user-agent", Utils.userAgent);
     }
 
     if (this.authenticator) {
       this.authenticator(this);
     } else {
       this._log.debug("No authenticator found.");
     }
 
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -1238,17 +1238,17 @@ Sync11Service.prototype = {
     if (ignore && ignore.indexOf(reason) != -1)
       return "";
 
     return reason;
   },
 
   sync: function sync(engineNamesToSync) {
     let dateStr = new Date().toLocaleFormat(LOG_DATE_FORMAT);
-    this._log.debug("User-Agent: " + SyncStorageRequest.prototype.userAgent);
+    this._log.debug("User-Agent: " + Utils.userAgent);
     this._log.info("Starting sync at " + dateStr);
     this._catch(function () {
       // Make sure we're logged in.
       if (this._shouldLogin()) {
         this._log.debug("In sync: should login.");
         if (!this.login()) {
           this._log.debug("Not syncing: login returned false.");
           return;
--- a/services/sync/modules/util.js
+++ b/services/sync/modules/util.js
@@ -55,16 +55,35 @@ this.Utils = {
   makeHMACKey: CryptoUtils.makeHMACKey,
   makeHMACHasher: CryptoUtils.makeHMACHasher,
   hkdfExpand: CryptoUtils.hkdfExpand,
   pbkdf2Generate: CryptoUtils.pbkdf2Generate,
   deriveKeyFromPassphrase: CryptoUtils.deriveKeyFromPassphrase,
   getHTTPMACSHA1Header: CryptoUtils.getHTTPMACSHA1Header,
 
   /**
+   * The string to use as the base User-Agent in Sync requests.
+   * This string will look something like
+   *
+   *   Firefox/49.0a1 (Windows NT 6.1; WOW64; rv:46.0) FxSync/1.51.0.20160516142357.desktop
+   */
+  _userAgent: null,
+  get userAgent() {
+    if (!this._userAgent) {
+      let hph = Cc["@mozilla.org/network/protocol;1?name=http"].getService(Ci.nsIHttpProtocolHandler);
+      this._userAgent =
+        Services.appinfo.name + "/" + Services.appinfo.version +  // Product.
+        " (" + hph.oscpu + ")" +                                  // (oscpu)
+        " FxSync/" + WEAVE_VERSION + "." +                        // Sync.
+        Services.appinfo.appBuildID + ".";                        // Build.
+    }
+    return this._userAgent + Svc.Prefs.get("client.type", "desktop");
+  },
+
+  /**
    * Wrap a function to catch all exceptions and log them
    *
    * @usage MyObj._catch = Utils.catch;
    *        MyObj.foo = function() { this._catch(func)(); }
    *
    * Optionally pass a function which will be called if an
    * exception occurs.
    */
--- a/services/sync/tests/unit/test_resource_ua.js
+++ b/services/sync/tests/unit/test_resource_ua.js
@@ -2,16 +2,19 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/resource.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://testing-common/services/sync/utils.js");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
+
 // Tracking info/collections.
 var collectionsHelper = track_collections_helper();
 var collections = collectionsHelper.collections;
 
 var meta_global;
 var server;
 
 var expectedUA;
@@ -32,17 +35,20 @@ function run_test() {
   });
 
   ensureLegacyIdentityManager();
   setBasicCredentials("johndoe", "ilovejane");
   Service.serverURL = server.baseURI + "/";
   Service.clusterURL = server.baseURI + "/";
   _("Server URL: " + server.baseURI);
 
+  // Note this string is missing the trailing ".destkop" as the test
+  // adjusts the "client.type" pref where that portion comes from.
   expectedUA = Services.appinfo.name + "/" + Services.appinfo.version +
+               " (" + httpProtocolHandler.oscpu + ")" +
                " FxSync/" + WEAVE_VERSION + "." +
                Services.appinfo.appBuildID;
 
   run_next_test();
 }
 
 add_test(function test_fetchInfo() {
   _("Testing _fetchInfo.");
--- a/services/sync/tests/unit/test_service_getStorageInfo.js
+++ b/services/sync/tests/unit/test_service_getStorageInfo.js
@@ -2,16 +2,19 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://services-common/rest.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://testing-common/services/sync/utils.js");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
+
 var collections = {steam:  65.11328,
                    petrol: 82.488281,
                    diesel: 2.25488281};
 
 function run_test() {
   Log.repository.getLogger("Sync.Service").level = Log.Level.Trace;
   Log.repository.getLogger("Sync.StorageRequest").level = Log.Level.Trace;
   initTestLogging();
@@ -32,16 +35,17 @@ add_test(function test_success() {
     do_check_eq(error, null);
     do_check_true(Utils.deepEquals(info, collections));
 
     // Ensure that the request is sent off with the right bits.
     do_check_true(basic_auth_matches(handler.request,
                                      Service.identity.username,
                                      Service.identity.basicPassword));
     let expectedUA = Services.appinfo.name + "/" + Services.appinfo.version +
+                     " (" + httpProtocolHandler.oscpu + ")" +
                      " FxSync/" + WEAVE_VERSION + "." +
                      Services.appinfo.appBuildID + ".desktop";
     do_check_eq(handler.request.getHeader("User-Agent"), expectedUA);
 
     server.stop(run_next_test);
   });
   do_check_true(request instanceof RESTRequest);
 });
--- a/services/sync/tests/unit/test_syncstoragerequest.js
+++ b/services/sync/tests/unit/test_syncstoragerequest.js
@@ -3,30 +3,34 @@
 
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/rest.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://testing-common/services/sync/utils.js");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
+
 function run_test() {
   Log.repository.getLogger("Sync.RESTRequest").level = Log.Level.Trace;
   initTestLogging();
 
   ensureLegacyIdentityManager();
 
   run_next_test();
 }
 
 add_test(function test_user_agent_desktop() {
   let handler = httpd_handler(200, "OK");
   let server = httpd_setup({"/resource": handler});
 
   let expectedUA = Services.appinfo.name + "/" + Services.appinfo.version +
+                   " (" + httpProtocolHandler.oscpu + ")" +
                    " FxSync/" + WEAVE_VERSION + "." +
                    Services.appinfo.appBuildID + ".desktop";
 
   let request = new SyncStorageRequest(server.baseURI + "/resource");
   request.onComplete = function onComplete(error) {
     do_check_eq(error, null);
     do_check_eq(this.response.status, 200);
     do_check_eq(handler.request.getHeader("User-Agent"), expectedUA);
@@ -36,16 +40,17 @@ add_test(function test_user_agent_deskto
 });
 
 add_test(function test_user_agent_mobile() {
   let handler = httpd_handler(200, "OK");
   let server = httpd_setup({"/resource": handler});
 
   Svc.Prefs.set("client.type", "mobile");
   let expectedUA = Services.appinfo.name + "/" + Services.appinfo.version +
+                   " (" + httpProtocolHandler.oscpu + ")" +
                    " FxSync/" + WEAVE_VERSION + "." +
                    Services.appinfo.appBuildID + ".mobile";
 
   let request = new SyncStorageRequest(server.baseURI + "/resource");
   request.get(function (error) {
     do_check_eq(error, null);
     do_check_eq(this.response.status, 200);
     do_check_eq(handler.request.getHeader("User-Agent"), expectedUA);