Bug 629463: delete bad server-side clients records. r=philiKON a=beltzner
authorRichard Newman <rnewman@mozilla.com>
Tue, 01 Mar 2011 11:56:29 -0800
changeset 63270 76784798d9a7424a105caed70eae54cd3b798e8c
parent 63269 1a0094bc7086d77609c5626b726baa5dfe37ac0b
child 63271 a213e981b60620f0336f1e09323ac2e925b3762a
push id1
push userroot
push dateTue, 10 Dec 2013 15:46:25 +0000
reviewersphiliKON, beltzner
bugs629463
Bug 629463: delete bad server-side clients records. r=philiKON a=beltzner
services/sync/modules/engines.js
services/sync/modules/engines/clients.js
services/sync/tests/unit/head_http_server.js
services/sync/tests/unit/test_clients_engine.js
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -712,17 +712,17 @@ SyncEngine.prototype = {
       
       // Remember which records were processed
       handled.push(item.id);
 
       try {
         try {
           item.decrypt();
         } catch (ex if (Utils.isHMACMismatch(ex) &&
-                        this.handleHMACMismatch())) {
+                        this.handleHMACMismatch(item))) {
           // Let's try handling it.
           // If the callback returns true, try decrypting again, because
           // we've got new keys.
           this._log.info("Trying decrypt again...");
           item.decrypt();
         }       
       } catch (ex) {
         this._log.warn("Error decrypting record: " + Utils.exceptionStr(ex));
@@ -1078,12 +1078,12 @@ SyncEngine.prototype = {
     this.toFetch = [];
   },
 
   wipeServer: function wipeServer() {
     new Resource(this.engineURL).delete();
     this._resetClient();
   },
   
-  handleHMACMismatch: function handleHMACMismatch() {
+  handleHMACMismatch: function handleHMACMismatch(item) {
     return Weave.Service.handleHMACEvent();
   }
 };
--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -187,16 +187,30 @@ ClientEngine.prototype = {
   },
 
   // Treat reset the same as wiping for locally cached clients
   _resetClient: function _resetClient() this._wipeClient(),
 
   _wipeClient: function _wipeClient() {
     SyncEngine.prototype._resetClient.call(this);
     this._store.wipe();
+  },
+  
+  // Override the default behavior to delete bad records from the server.
+  handleHMACMismatch: function handleHMACMismatch(item) {
+    this._log.debug("Handling HMAC mismatch for " + item.id);
+    if (SyncEngine.prototype.handleHMACMismatch.call(this, item))
+      return true;
+
+    // It's a bad client record. Save it to be deleted at the end of the sync.
+    this._log.debug("Bad client record detected. Scheduling for deletion.");
+    this._deleteId(item.id);
+
+    // Don't try again.
+    return false;
   }
 };
 
 function ClientStore(name) {
   Store.call(this, name);
 }
 ClientStore.prototype = {
   __proto__: Store.prototype,
--- a/services/sync/tests/unit/head_http_server.js
+++ b/services/sync/tests/unit/head_http_server.js
@@ -134,16 +134,27 @@ function ServerCollection(wbos, acceptNe
 ServerCollection.prototype = {
 
   _inResultSet: function(wbo, options) {
     return wbo.payload
            && (!options.ids || (options.ids.indexOf(wbo.id) != -1))
            && (!options.newer || (wbo.modified > options.newer));
   },
 
+  count: function(options) {
+    options = options || {};
+    let c = 0;
+    for (let [id, wbo] in Iterator(this.wbos)) {
+      if (wbo.modified && this._inResultSet(wbo, options)) {
+        c++;
+      }
+    }
+    return c;
+  },
+
   get: function(options) {
     let result;
     if (options.full) {
       let data = [wbo.get() for ([id, wbo] in Iterator(this.wbos))
                             // Drop deleted.
                             if (wbo.modified &&
                                 this._inResultSet(wbo, options))];
       if (options.limit) {
--- a/services/sync/tests/unit/test_clients_engine.js
+++ b/services/sync/tests/unit/test_clients_engine.js
@@ -1,17 +1,93 @@
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/record.js");
 Cu.import("resource://services-sync/identity.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://services-sync/engines/clients.js");
+Cu.import("resource://services-sync/service.js");
 
 const MORE_THAN_CLIENTS_TTL_REFRESH = 691200; // 8 days
 const LESS_THAN_CLIENTS_TTL_REFRESH = 86400; // 1 day
 
+function test_bad_hmac() {
+  _("Ensure that Clients engine deletes corrupt records.");
+  let global = new ServerWBO('global',
+                             {engines: {clients: {version: Clients.version,
+                                                  syncID: Clients.syncID}}});
+  let clientsColl = new ServerCollection({}, true);
+  let keysWBO = new ServerWBO("keys");
+
+  let collectionsHelper = track_collections_helper();
+  let upd = collectionsHelper.with_updated_collection;
+  let collections = collectionsHelper.collections;
+
+  // Watch for deletions in the given collection.
+  let deleted = false;
+  function trackDeletedHandler(coll, handler) {
+    let u = upd(coll, handler);
+    return function(request, response) {
+      if (request.method == "DELETE")
+        deleted = true;
+
+      return u(request, response);
+    };
+  }
+
+  let handlers = {
+    "/1.0/foo/info/collections": collectionsHelper.handler,
+    "/1.0/foo/storage/meta/global": upd("meta", global.handler()),
+    "/1.0/foo/storage/crypto/keys": upd("crypto", keysWBO.handler()),
+    "/1.0/foo/storage/clients": trackDeletedHandler("crypto", clientsColl.handler())
+  };
+
+  let server = httpd_setup(handlers);
+  do_test_pending();
+
+  try {
+    let passphrase = "abcdeabcdeabcdeabcdeabcdea";
+    Service.serverURL = "http://localhost:8080/";
+    Service.clusterURL = "http://localhost:8080/";
+    Service.login("foo", "ilovejane", passphrase);
+
+    CollectionKeys.generateNewKeys();
+
+    _("First sync, client record is uploaded");
+    do_check_eq(0, clientsColl.count());
+    do_check_eq(Clients.lastRecordUpload, 0);
+    Clients.sync();
+    do_check_eq(1, clientsColl.count());
+    do_check_true(Clients.lastRecordUpload > 0);
+    deleted = false;    // Initial setup can wipe the server, so clean up.
+
+    _("Records now: " + clientsColl.get({}));
+    _("Change our keys and our client ID, reupload keys.");
+    Clients.localID = Utils.makeGUID();
+    Clients.resetClient();
+    CollectionKeys.generateNewKeys();
+    let serverKeys = CollectionKeys.asWBO("crypto", "keys");
+    serverKeys.encrypt(Weave.Service.syncKeyBundle);
+    do_check_true(serverKeys.upload(Weave.Service.cryptoKeysURL).success);
+
+    _("Sync.");
+    do_check_true(!deleted);
+    Clients.sync();
+
+    _("Old record was deleted, new one uploaded.");
+    do_check_true(deleted);
+    do_check_eq(1, clientsColl.count());
+    _("Records now: " + clientsColl.get({}));
+
+  } finally {
+    server.stop(do_test_finished);
+    Svc.Prefs.resetBranch("");
+    Records.clearCache();
+  }
+}
+
 function test_properties() {
   try {
     _("Test lastRecordUpload property");
     do_check_eq(Svc.Prefs.get("clients.lastRecordUpload"), undefined);
     do_check_eq(Clients.lastRecordUpload, 0);
 
     let now = Date.now();
     Clients.lastRecordUpload = now / 1000;
@@ -69,11 +145,14 @@ function test_sync() {
     server.stop(do_test_finished);
     Svc.Prefs.resetBranch("");
     Records.clearCache();
   }
 }
 
 
 function run_test() {
+  initTestLogging("Trace");
+  Log4Moz.repository.getLogger("Engine.Clients").level = Log4Moz.Level.Trace;
+  test_bad_hmac();      // Needs to run first: doesn't use fake service!
   test_properties();
   test_sync();
 }