Bug 1217065 - Unconditionally ack incoming updates. r=dragana,benbangert
authorKit Cambridge <kcambridge@mozilla.com>
Thu, 22 Oct 2015 10:32:33 -0600
changeset 304155 97c0692a9ce5336fb5579aa52581fba8eb79c32a
parent 304154 f0134eb0b78f2145a778bcc790c416102225618b
child 304156 b5fc7e286929c74bb3e4d2a9cd8d6d9b36b97356
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana, benbangert
bugs1217065
milestone44.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 1217065 - Unconditionally ack incoming updates. r=dragana,benbangert
dom/push/PushDB.jsm
dom/push/PushServiceWebSocket.jsm
dom/push/test/xpcshell/test_notification_data.js
dom/push/test/xpcshell/xpcshell.ini
--- a/dom/push/PushDB.jsm
+++ b/dom/push/PushDB.jsm
@@ -394,17 +394,18 @@ this.PushDB.prototype = {
 
             let record = aEvent.target.result;
             if (!record) {
               debug("update: Key ID " + aKeyID + " does not exist");
               return;
             }
             let newRecord = aUpdateFunc(this.toPushRecord(record));
             if (!this.isValidRecord(newRecord)) {
-              debug("update: Ignoring invalid update for key ID " + aKeyID);
+              debug("update: Ignoring invalid update for key ID " + aKeyID +
+                ": " + JSON.stringify(newRecord));
               return;
             }
             aStore.put(newRecord).onsuccess = aEvent => {
               debug("update: Update successful for key ID " + aKeyID);
               aTxn.result = newRecord;
             };
           };
         },
--- a/dom/push/PushServiceWebSocket.jsm
+++ b/dom/push/PushServiceWebSocket.jsm
@@ -888,16 +888,25 @@ this.PushServiceWebSocket = {
   },
 
   _handleDataUpdate: function(update) {
     let promise;
     if (typeof update.channelID != "string") {
       debug("handleDataUpdate: Discarding message without channel ID");
       return;
     }
+    // Unconditionally ack the update. This is important because the Push
+    // server requires the client to ack all outstanding updates before
+    // resuming delivery. However, the server doesn't verify the encryption
+    // params, and can't ensure that an update is encrypted correctly because
+    // it doesn't have the private key. Thus, if we only acked valid updates,
+    // it would be possible for a single invalid one to block delivery of all
+    // subsequent updates. A nack would be more appropriate for this case, but
+    // the protocol doesn't currently support them.
+    this._sendAck(update.channelID, update.version);
     if (typeof update.data != "string") {
       promise = this._mainPushService.receivedPushMessage(
         update.channelID,
         null,
         null,
         record => record
       );
     } else {
@@ -909,17 +918,17 @@ this.PushServiceWebSocket = {
       let message = base64UrlDecode(update.data);
       promise = this._mainPushService.receivedPushMessage(
         update.channelID,
         message,
         params,
         record => record
       );
     }
-    promise.then(() => this._sendAck(update.channelID)).catch(err => {
+    promise.catch(err => {
       debug("handleDataUpdate: Error delivering message: " + err);
     });
   },
 
   /**
    * Protocol handler invoked by server message.
    */
   _handleNotificationReply: function(reply) {
new file mode 100644
--- /dev/null
+++ b/dom/push/test/xpcshell/test_notification_data.js
@@ -0,0 +1,198 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+'use strict';
+
+const {PushDB, PushService, PushServiceWebSocket} = serviceExports;
+
+let db;
+let userAgentID = 'f5b47f8d-771f-4ea3-b999-91c135f8766d';
+
+function run_test() {
+  do_get_profile();
+  setPrefs({
+    userAgentID: userAgentID,
+  });
+  run_next_test();
+}
+
+function putRecord(channelID, scope, publicKey, privateKey) {
+  return db.put({
+    channelID: channelID,
+    pushEndpoint: 'https://example.org/push/' + channelID,
+    scope: scope,
+    pushCount: 0,
+    lastPush: 0,
+    originAttributes: '',
+    quota: Infinity,
+    p256dhPublicKey: publicKey,
+    p256dhPrivateKey: privateKey,
+  });
+}
+
+add_task(function* test_notification_ack_data() {
+  db = PushServiceWebSocket.newPushDB();
+  do_register_cleanup(() => {return db.drop().then(_ => db.close());});
+
+  yield putRecord(
+    'subscription1',
+    'https://example.com/page/1',
+    'BPCd4gNQkjwRah61LpdALdzZKLLnU5UAwDztQ5_h0QsT26jk0IFbqcK6-JxhHAm-rsHEwy0CyVJjtnfOcqc1tgA',
+    {
+      crv: 'P-256',
+      d: '1jUPhzVsRkzV0vIzwL4ZEsOlKdNOWm7TmaTfzitJkgM',
+      ext: true,
+      key_ops: ["deriveBits"],
+      kty: "EC",
+      x: '8J3iA1CSPBFqHrUul0At3NkosudTlQDAPO1Dn-HRCxM',
+      y: '26jk0IFbqcK6-JxhHAm-rsHEwy0CyVJjtnfOcqc1tgA'
+    }
+  );
+  yield putRecord(
+    'subscription2',
+    'https://example.com/page/2',
+    'BPnWyUo7yMnuMlyKtERuLfWE8a09dtdjHSW2lpC9_BqR5TZ1rK8Ldih6ljyxVwnBA-nygQHGRpEmu1jV5K8437E',
+    {
+      crv: 'P-256',
+      d: 'lFm4nPsUKYgNGBJb5nXXKxl8bspCSp0bAhCYxbveqT4',
+      ext: true,
+      key_ops: ["deriveBits"],
+      kty: 'EC',
+      x: '-dbJSjvIye4yXIq0RG4t9YTxrT1212MdJbaWkL38GpE',
+      y: '5TZ1rK8Ldih6ljyxVwnBA-nygQHGRpEmu1jV5K8437E'
+    }
+  );
+  yield putRecord(
+    'subscription3',
+    'https://example.com/page/3',
+    'BDhUHITSeVrWYybFnb7ylVTCDDLPdQWMpf8gXhcWwvaaJa6n3YH8TOcH8narDF6t8mKVvg2ioLW-8MH5O4dzGcI',
+    {
+      crv: 'P-256',
+      d: 'Q1_SE1NySTYzjbqgWwPgrYh7XRg3adqZLkQPsy319G8',
+      ext: true,
+      key_ops: ["deriveBits"],
+      kty: 'EC',
+      x: 'OFQchNJ5WtZjJsWdvvKVVMIMMs91BYyl_yBeFxbC9po',
+      y: 'Ja6n3YH8TOcH8narDF6t8mKVvg2ioLW-8MH5O4dzGcI'
+    }
+  );
+
+  let updates = [];
+  let notifyPromise = promiseObserverNotification('push-notification', function(subject, data) {
+    let notification = subject.QueryInterface(Ci.nsIPushObserverNotification);
+    updates.push({
+      scope: data,
+      data: notification.data,
+    });
+    return updates.length == 3;
+  });
+
+  let acks = 0;
+  let ackDone;
+  let ackPromise = new Promise(resolve => ackDone = resolve);
+  PushService.init({
+    serverURI: "wss://push.example.org/",
+    networkInfo: new MockDesktopNetworkInfo(),
+    db,
+    makeWebSocket(uri) {
+      return new MockWebSocket(uri, {
+        onHello(request) {
+          equal(request.uaid, userAgentID,
+            'Should send matching device IDs in handshake');
+          deepEqual(
+            request.channelIDs.sort(),
+            ['subscription1', 'subscription2', 'subscription3'],
+            'Should send matching channel IDs in handshake'
+          );
+          this.serverSendMsg(JSON.stringify({
+            messageType: 'hello',
+            uaid: userAgentID,
+            status: 200,
+            use_webpush: true,
+          }));
+          // subscription1 will send a message with no rs and padding
+          // length 1.
+          this.serverSendMsg(JSON.stringify({
+            messageType: 'notification',
+            channelID: 'subscription1',
+            headers: {
+              encryption_key: 'keyid="notification1"; dh="BO_tgGm-yvYAGLeRe16AvhzaUcpYRiqgsGOlXpt0DRWDRGGdzVLGlEVJMygqAUECarLnxCiAOHTP_znkedrlWoU"',
+              encryption: 'keyid="notification1";salt="uAZaiXpOSfOLJxtOCZ09dA"',
+            },
+            data: 'NwrrOWPxLE8Sv5Rr0Kep7n0-r_j3rsYrUw_CXPo',
+            version: 'v1',
+          }));
+        },
+        onACK(request) {
+          switch (++acks) {
+          case 1:
+            deepEqual([{
+              channelID: 'subscription1',
+              version: 'v1',
+            }], request.updates, 'Wrong updates for acknowledgement 1');
+            // subscription2 will send a message with no rs and padding
+            // length 16.
+            this.serverSendMsg(JSON.stringify({
+              messageType: 'notification',
+              channelID: 'subscription2',
+              headers: {
+                encryption_key: 'keyid="notification2"; dh="BKVdQcgfncpNyNWsGrbecX0zq3eHIlHu5XbCGmVcxPnRSbhjrA6GyBIeGdqsUL69j5Z2CvbZd-9z1UBH0akUnGQ"',
+                encryption: 'keyid="notification2";salt="vFn3t3M_k42zHBdpch3VRw"',
+              },
+              data: 'Zt9dEdqgHlyAL_l83385aEtb98ZBilz5tgnGgmwEsl5AOCNgesUUJ4p9qUU',
+              version: 'v2',
+            }));
+            break;
+
+          case 2:
+            deepEqual([{
+              channelID: 'subscription2',
+              version: 'v2',
+            }], request.updates, 'Wrong updates for acknowledgement 2');
+            // subscription3 will send a message with rs equal 24 and
+            // padding length 16.
+            this.serverSendMsg(JSON.stringify({
+              messageType: 'notification',
+              channelID: 'subscription3',
+              headers: {
+                encryption_key: 'keyid="notification3";dh="BD3xV_ACT8r6hdIYES3BJj1qhz9wyv7MBrG9vM2UCnjPzwE_YFVpkD-SGqE-BR2--0M-Yf31wctwNsO1qjBUeMg"',
+                encryption: 'keyid="notification3"; salt="DFq188piWU7osPBgqn4Nlg"; rs=24',
+              },
+              data: 'LKru3ZzxBZuAxYtsaCfaj_fehkrIvqbVd1iSwnwAUgnL-cTeDD-83blxHXTq7r0z9ydTdMtC3UjAcWi8LMnfY-BFzi0qJAjGYIikDA',
+              version: 'v3',
+            }));
+            break;
+
+          case 3:
+            deepEqual([{
+              channelID: 'subscription3',
+              version: 'v3',
+            }], request.updates, 'Wrong updates for acknowledgement 3');
+            ackDone();
+            break;
+
+          default:
+            ok(false, 'Unexpected acknowledgement ' + acks);
+          }
+        }
+      });
+    }
+  });
+
+  yield waitForPromise(notifyPromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for notifications');
+  yield waitForPromise(ackPromise, DEFAULT_TIMEOUT,
+    'Timed out waiting for multiple acknowledgements');
+
+  updates.sort((a, b) => a.scope < b.scope ? -1 : a.scope > b.scope ? 1 : 0);
+  deepEqual([{
+    scope: 'https://example.com/page/1',
+    data: 'Some message',
+  }, {
+    scope: 'https://example.com/page/2',
+    data: 'Some message',
+  }, {
+    scope: 'https://example.com/page/3',
+    data: 'Some message',
+  }], updates, 'Wrong data for notifications');
+});
--- a/dom/push/test/xpcshell/xpcshell.ini
+++ b/dom/push/test/xpcshell/xpcshell.ini
@@ -1,15 +1,16 @@
 [DEFAULT]
 head = head.js head-http2.js
 tail =
 # Push notifications and alarms are currently disabled on Android.
 skip-if = toolkit == 'android'
 
 [test_notification_ack.js]
+[test_notification_data.js]
 [test_notification_duplicate.js]
 [test_notification_error.js]
 [test_notification_incomplete.js]
 [test_notification_version_string.js]
 
 [test_permissions.js]
 run-sequentially = This will delete all existing push subscriptions.