Merge pull request #95 from amplitude/limit_property_keys

Limit property keys

Author: djih

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Unreleased
 
+* Block event property and user property dictionaries that have more than 1000 items. This is to block properties that are set unintentionally (for example in a loop). A single call to `logEvent` should not have more than 1000 event properties. Similarly a single call to `setUserProperties` should not have more than 1000 user properties.
+
+### 3.1.0 (September 14, 2016)
+
 * Add configuration option `forceHttps`, which when set to `true` forces the SDK to always upload to HTTPS endpoint. By default the SDK uses the endpoint that matches the embedding site's protocol (for example if your site is HTTP, it will use the HTTP endpoint).
 
 ### 3.0.2 (July 6, 2016)

README.md

@@ -124,7 +124,9 @@ Having large amounts of distinct event types, event properties and user properti
   * 2000 distinct event properties
   * 1000 distinct user properties
 
-Anything past the above thresholds will not be visualized. **Note that the raw data is not impacted by this in any way, meaning you can still see the values in the raw data, but they will not be visualized on the platform.** We have put in very conservative estimates for the event and property caps which we don’t expect to be exceeded in any practical use case. If you feel that your use case will go above those limits please reach out to [email protected].
+Anything past the above thresholds will not be visualized. **Note that the raw data is not impacted by this in any way, meaning you can still see the values in the raw data, but they will not be visualized on the platform.**
+
+A single call to `logEvent` should not have more than 1000 event properties. Likewise a single call to `setUserProperties` should not have more than 1000 user properties. If the 1000 item limit is exceeded then the properties will be dropped and a warning will be printed to console. We have put in very conservative estimates for the event and property caps which we don’t expect to be exceeded in any practical use case. If you feel that your use case will go above those limits please reach out to [email protected].
 
 # Setting Custom User IDs #
 

amplitude.js

@@ -1134,11 +1134,17 @@ AmplitudeClient.prototype.setUserProperties = function setUserProperties(userPro
   if (!this._apiKeySet('setUserProperties()') || !utils.validateInput(userProperties, 'userProperties', 'object')) {
     return;
   }
+  // sanitize the userProperties dict before converting into identify
+  var sanitized = utils.truncate(utils.validateProperties(userProperties));
+  if (Object.keys(sanitized).length === 0) {
+    return;
+  }
+
   // convert userProperties into an identify call
   var identify = new Identify();
-  for (var property in userProperties) {
-    if (userProperties.hasOwnProperty(property)) {
-      identify.set(property, userProperties[property]);
+  for (var property in sanitized) {
+    if (sanitized.hasOwnProperty(property)) {
+      identify.set(property, sanitized[property]);
     }
   }
   this.identify(identify);
@@ -1618,6 +1624,7 @@ module.exports = {
   DEFAULT_INSTANCE: '$default_instance',
   API_VERSION: 2,
   MAX_STRING_LENGTH: 4096,
+  MAX_PROPERTY_KEYS: 1000,
   IDENTIFY_EVENT: '$identify',
 
   // localStorageKeys
@@ -2725,10 +2732,16 @@ var validateInput = function validateInput(input, name, expectedType) {
   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);
   if (propsType !== 'object') {
-    log('Error: invalid event properties format. Expecting Javascript object, received ' + propsType + ', ignoring');
+    log('Error: invalid properties format. Expecting Javascript object, received ' + propsType + ', ignoring');
+    return {};
+  }
+
+  if (Object.keys(properties).length > constants.MAX_PROPERTY_KEYS) {
+    log('Error: too many properties (more than 1000), ignoring');
     return {};
   }
 

amplitude.min.js

@@ -653,11 +653,17 @@ AmplitudeClient.prototype.setUserProperties = function setUserProperties(userPro
   if (!this._apiKeySet('setUserProperties()') || !utils.validateInput(userProperties, 'userProperties', 'object')) {
     return;
   }
+  // sanitize the userProperties dict before converting into identify
+  var sanitized = utils.truncate(utils.validateProperties(userProperties));
+  if (Object.keys(sanitized).length === 0) {
+    return;
+  }
+
   // convert userProperties into an identify call
   var identify = new Identify();
-  for (var property in userProperties) {
-    if (userProperties.hasOwnProperty(property)) {
-      identify.set(property, userProperties[property]);
+  for (var property in sanitized) {
+    if (sanitized.hasOwnProperty(property)) {
+      identify.set(property, sanitized[property]);
     }
   }
   this.identify(identify);

src/amplitude-client.js

@@ -2,6 +2,7 @@ module.exports = {
   DEFAULT_INSTANCE: '$default_instance',
   API_VERSION: 2,
   MAX_STRING_LENGTH: 4096,
+  MAX_PROPERTY_KEYS: 1000,
   IDENTIFY_EVENT: '$identify',
 
   // localStorageKeys

src/constants.js

@@ -56,10 +56,16 @@ var validateInput = function validateInput(input, name, expectedType) {
   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);
   if (propsType !== 'object') {
-    log('Error: invalid event properties format. Expecting Javascript object, received ' + propsType + ', ignoring');
+    log('Error: invalid properties format. Expecting Javascript object, received ' + propsType + ', ignoring');
+    return {};
+  }
+
+  if (Object.keys(properties).length > constants.MAX_PROPERTY_KEYS) {
+    log('Error: too many properties (more than 1000), ignoring');
     return {};
   }
 

src/utils.js

@@ -11,6 +11,7 @@ describe('AmplitudeClient', function() {
   var JSON = require('json');
   var Identify = require('../src/identify.js');
   var Revenue = require('../src/revenue.js');
+  var constants = require('../src/constants.js');
   var apiKey = '000000';
   var keySuffix = '_' + apiKey.slice(0,6);
   var userId = 'user';
@@ -1921,14 +1922,46 @@ describe('setVersionName', function() {
       });
     });
 
-    it('should validate user propeorties', function() {
+    it('should validate user properties', function() {
       var identify = new Identify().set(10, 10);
       amplitude.init(apiKey, null, {batchEvents: true});
       amplitude.identify(identify);
 
       assert.deepEqual(amplitude._unsentIdentifys[0].user_properties, {'$set': {'10': 10}});
     });
 
+    it('should ignore event and user properties with too many items', function() {
+      amplitude.init(apiKey, null, {batchEvents: true, eventUploadThreshold: 2});
+      var eventProperties = {};
+      var userProperties = {};
+      var identify = new Identify();
+      for (var i = 0; i < constants.MAX_PROPERTY_KEYS + 1; i++) {
+        eventProperties[i] = i;
+        userProperties[i*2] = i*2;
+        identify.set(i, i);
+      }
+
+      // verify that setUserProperties ignores the dict completely
+      amplitude.setUserProperties(userProperties);
+      assert.lengthOf(amplitude._unsentIdentifys, 0);
+      assert.lengthOf(server.requests, 0);
+
+      // verify that the event properties and user properties are scrubbed
+      amplitude.logEvent('test event', eventProperties);
+      amplitude.identify(identify);
+
+      assert.lengthOf(server.requests, 1);
+      var events = JSON.parse(querystring.parse(server.requests[0].requestBody).e);
+      assert.lengthOf(events, 2);
+
+      assert.equal(events[0].event_type, 'test event');
+      assert.deepEqual(events[0].event_properties, {});
+      assert.deepEqual(events[0].user_properties, {});
+      assert.equal(events[1].event_type, '$identify');
+      assert.deepEqual(events[1].event_properties, {});
+      assert.deepEqual(events[1].user_properties, {'$set': {}});
+    });
+
     it('should synchronize event data across multiple amplitude instances that share the same cookie', function() {
       // this test fails if logEvent does not reload cookie data every time
       var amplitude1 = new AmplitudeClient();

test/amplitude-client.js

@@ -1,5 +1,6 @@
 describe('utils', function() {
   var utils = require('../src/utils.js');
+  var constants = require('../src/constants.js');
 
   describe('isEmptyString', function() {
     it('should detect empty strings', function() {
@@ -123,5 +124,13 @@ describe('utils', function() {
       }
       assert.deepEqual(utils.validateProperties(properties), expected);
     });
+
+    it('should block properties with too many items', function() {
+      var properties = {};
+      for (var i = 0; i < constants.MAX_PROPERTY_KEYS + 1; i++) {
+        properties[i] = i;
+      }
+      assert.deepEqual(utils.validateProperties(properties), {});
+    });
   });
 });