fix: Prevent periods in device id check (#405)

* Add validateDeviceId function

* Add device id check during init

* add tests

* fix test

* deviceId includes nit

* extra test

Author: jooohhn

src/amplitude-client.js

@@ -148,6 +148,12 @@ AmplitudeClient.prototype.init = function init(apiKey, opt_userId, opt_config, o
     this._pendingReadStorage = true;
 
     const initFromStorage = (storedDeviceId) => {
+      if (opt_config && opt_config.deviceId && !utils.validateDeviceId(opt_config.deviceId)) {
+        utils.log.error(
+          `Invalid device ID rejected. Randomly generated UUID will be used instead of "${opt_config.deviceId}"`,
+        );
+        delete opt_config.deviceId;
+      }
       this.options.deviceId = this._getInitialDeviceId(opt_config && opt_config.deviceId, storedDeviceId);
       this.options.userId =
         (type(opt_userId) === 'string' && !utils.isEmptyString(opt_userId) && opt_userId) ||
@@ -927,8 +933,9 @@ AmplitudeClient.prototype.regenerateDeviceId = function regenerateDeviceId() {
 };
 
 /**
- * Sets a custom deviceId for current user. Note: this is not recommended unless you know what you are doing
- * (like if you have your own system for managing deviceIds). Make sure the deviceId you set is sufficiently unique
+ * Sets a custom deviceId for current user. **Values may not have `.` inside them**
+ * Note: this is not recommended unless you know what you are doing (like if you have your own system for managing deviceIds).
+ * Make sure the deviceId you set is sufficiently unique
  * (we recommend something like a UUID - see src/uuid.js for an example of how to generate) to prevent conflicts with other devices in our system.
  * @public
  * @param {string} deviceId - custom deviceId for current user.
@@ -939,7 +946,7 @@ AmplitudeClient.prototype.setDeviceId = function setDeviceId(deviceId) {
     return this._q.push(['setDeviceId'].concat(Array.prototype.slice.call(arguments, 0)));
   }
 
-  if (!utils.validateInput(deviceId, 'deviceId', 'string')) {
+  if (!utils.validateDeviceId(deviceId)) {
     return;
   }
 

src/utils.js

@@ -97,6 +97,17 @@ var validateInput = function validateInput(input, name, expectedType) {
   return true;
 };
 
+const validateDeviceId = function validateDeviceId(deviceId) {
+  if (!validateInput(deviceId, 'deviceId', 'string')) {
+    return false;
+  }
+  if (deviceId.includes('.')) {
+    log.error(`Device IDs may not contain '.' characters. Value will be ignored: "${deviceId}"`);
+    return false;
+  }
+  return true;
+};
+
 // do some basic sanitization and type checking, also catch property dicts with more than 1000 key/value pairs
 var validateProperties = function validateProperties(properties) {
   var propsType = type(properties);
@@ -257,4 +268,5 @@ export default {
   validateGroups,
   validateInput,
   validateProperties,
+  validateDeviceId,
 };

test/amplitude-client.js

@@ -285,6 +285,26 @@ describe('AmplitudeClient', function () {
       amplitude._getUrlParams.restore();
     });
 
+    it('should reject invalid device ids that contain periods', function () {
+      const spyErrorWarning = sinon.spy(utils.log, 'error');
+      const badDeviceId = 'bad.device.id';
+      amplitude.init(apiKey, null, { deviceId: badDeviceId });
+
+      assert.isTrue(
+        spyErrorWarning.calledWith(
+          `Device IDs may not contain '.' characters. Value will be ignored: "${badDeviceId}"`,
+        ),
+      );
+      assert.isTrue(
+        spyErrorWarning.calledWith(
+          `Invalid device ID rejected. Randomly generated UUID will be used instead of "${badDeviceId}"`,
+        ),
+      );
+      assert.notEqual(amplitude.options.deviceId, badDeviceId);
+
+      spyErrorWarning.restore();
+    });
+
     it('should load device id from the cookie', function () {
       // deviceId and sequenceNumber not set, init should load value from localStorage
       var cookieData = {
@@ -1048,6 +1068,21 @@ describe('AmplitudeClient', function () {
       var stored = amplitude._metadataStorage.load();
       assert.propertyVal(stored, 'deviceId', 'deviceId');
     });
+
+    it('should not take periods in deviceId', function () {
+      const spyErrorWarning = sinon.spy(utils.log, 'error');
+      amplitude.init(apiKey, null, { deviceId: 'fakeDeviceId' });
+      const badDeviceId = 'bad.device.id';
+      amplitude.setDeviceId(badDeviceId);
+      var stored = amplitude._metadataStorage.load();
+      assert.propertyVal(stored, 'deviceId', 'fakeDeviceId');
+      assert.isTrue(
+        spyErrorWarning.calledWith(
+          `Device IDs may not contain '.' characters. Value will be ignored: "${badDeviceId}"`,
+        ),
+      );
+      spyErrorWarning.restore();
+    });
   });
 
   describe('resetSessionId', function () {