Reject non-string installationId; add tests

Prevent non-string installationId values from causing a server error by validating the type in RestWrite: remove invalid installationId on create so the existing "must specify ID" check runs, and throw Parse.Error.INVALID_JSON when installationId exists but is not a string (avoids crashes from .toLowerCase()). Add unit tests to cover create/update failures for non-string installationId inputs and ensure master key cannot clear installationId via null.

Author: AdrianCurtin

spec/ParseInstallation.spec.js

@@ -636,7 +636,53 @@ describe('Installations', () => {
       });
   });
 
-  it('master key cannot clear installationId', done => {
+  it('create fails when installationId is a non-string object', done => {
+    const input = {
+      installationId: { foo: 'bar' },
+      deviceType: 'ios',
+    };
+    rest
+      .create(config, auth.nobody(config), '_Installation', input)
+      .then(() => {
+        fail('Creating the installation should have failed.');
+        done();
+      })
+      .catch(error => {
+        expect(error.code).toEqual(Parse.Error.INVALID_JSON);
+        done();
+      });
+  });
+
+  it('update fails when installationId is a non-string object', done => {
+    const installId = '12345678-abcd-abcd-abcd-123456789abc';
+    const input = {
+      installationId: installId,
+      deviceType: 'ios',
+    };
+    rest
+      .create(config, auth.nobody(config), '_Installation', input)
+      .then(() => database.adapter.find('_Installation', installationSchema, {}, {}))
+      .then(results => {
+        expect(results.length).toEqual(1);
+        return rest.update(
+          config,
+          auth.nobody(config),
+          '_Installation',
+          { objectId: results[0].objectId },
+          { installationId: { foo: 'bar' } }
+        );
+      })
+      .then(() => {
+        fail('Updating the installation should have failed.');
+        done();
+      })
+      .catch(error => {
+        expect(error.code).toEqual(Parse.Error.INVALID_JSON);
+        done();
+      });
+  });
+
+  it('master key cannot clear installationId via Delete op', done => {
     const installId = '12345678-abcd-abcd-abcd-123456789abc';
     const input = {
       installationId: installId,
@@ -665,6 +711,35 @@ describe('Installations', () => {
       });
   });
 
+  it('master key cannot clear installationId via null', done => {
+    const installId = '12345678-abcd-abcd-abcd-123456789abc';
+    const input = {
+      installationId: installId,
+      deviceType: 'ios',
+    };
+    rest
+      .create(config, auth.master(config), '_Installation', input)
+      .then(() => database.adapter.find('_Installation', installationSchema, {}, {}))
+      .then(results => {
+        expect(results.length).toEqual(1);
+        return rest.update(
+          config,
+          auth.master(config),
+          '_Installation',
+          { objectId: results[0].objectId },
+          { installationId: null }
+        );
+      })
+      .then(() => {
+        fail('Master key clearing of installationId via null should have been rejected.');
+        done();
+      })
+      .catch(error => {
+        expect(error.code).toEqual(136);
+        done();
+      });
+  });
+
   it('update fails to change deviceType', done => {
     const installId = '12345678-abcd-abcd-abcd-123456789abc';
     let input = {

src/RestWrite.js

@@ -1309,11 +1309,23 @@ RestWrite.prototype.handleInstallation = function () {
     if (this.query) {
       throw new Parse.Error(136, 'installationId may not be changed in this operation');
     }
-    // Create path: drop the operator/null so the "must specify ID"
-    // guard below fires with the correct 135 error.
+    // Create path: drop the invalid value so the existing "must specify
+    // ID" guard below can run. If no alternative ID (deviceToken,
+    // auth.installationId) is supplied the create is rejected with
+    // error 135; otherwise the create proceeds with the remaining ID.
     delete this.data.installationId;
   }
 
+  // Any remaining non-string installationId (object, array, number, etc.)
+  // would crash on `.toLowerCase()` below; reject it as a Parse error
+  // rather than letting it surface as a 500.
+  if (
+    this.data.installationId !== undefined &&
+    typeof this.data.installationId !== 'string'
+  ) {
+    throw new Parse.Error(Parse.Error.INVALID_JSON, 'installationId must be a string');
+  }
+
   if (
     !this.query &&
     !this.data.deviceToken &&