Bug 1273587 - Change the User-Agent header sent to Sync to include OS information. r?kitcambridge draft
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 31 May 2016 09:25:10 +1000
changeset 373060 40633fda7d64cf2514d4dbf59adc9ab9039ac9f3
parent 370796 788365ddaf61746bc66479335ae72d3e566c38ec
child 522318 96915c0f79f37896c38350245cf338af02f40638
push id19670
push userbmo:markh@mozilla.com
push dateMon, 30 May 2016 23:28:29 +0000
reviewerskitcambridge
bugs1273587
milestone49.0a1
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);