feat(attribute_value): Don't target NAN, INF, -INF and > 2^53. (#204)

mfahadahmed · mikeproeng37 · commit 7fe2e0ec13f3 · 2019-01-08T11:06:40.000-08:00
diff --git a/packages/optimizely-sdk/lib/core/custom_attribute_condition_evaluator/index.tests.js b/packages/optimizely-sdk/lib/core/custom_attribute_condition_evaluator/index.tests.js
@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2018, Optimizely, Inc. and contributors                        *
+ * Copyright 2018-2019, Optimizely, Inc. and contributors                        *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -200,6 +200,10 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
         };
         var result = customAttributeEvaluator.evaluate(invalidValueCondition, { lasers_count: 9000 });
         assert.isNull(result);
+
+        invalidValueCondition.value = Math.pow(2, 53) + 2;
+        result = customAttributeEvaluator.evaluate(invalidValueCondition, { lasers_count: 9000 });
+        assert.isNull(result);
       });
     });
 
@@ -305,6 +309,11 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
         meters_travelled: Infinity,
       });
       assert.isNull(result);
+
+      result = customAttributeEvaluator.evaluate(gtCondition, {
+        meters_travelled: Math.pow(2, 53) + 2,
+      });
+      assert.isNull(result);
     });
 
     it('should return null if there is no user-provided value', function() {
@@ -326,6 +335,11 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
       invalidValueCondition.value = null;
       result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
       assert.isNull(result);
+
+      invalidValueCondition.value = Math.pow(2, 53) + 2;
+      result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
+      assert.isNull(result);
+
     });
   });
 
@@ -366,6 +380,11 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
         meters_travelled: Infinity,
       });
       assert.isNull(result);
+
+      result = customAttributeEvaluator.evaluate(ltCondition, {
+        meters_travelled: Math.pow(2, 53) + 2,
+      });
+      assert.isNull(result);
     });
 
     it('should return null if there is no user-provided value', function() {
@@ -387,6 +406,10 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
       invalidValueCondition.value = {};
       result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
       assert.isNull(result);
+
+      invalidValueCondition.value = Math.pow(2, 53) + 2;
+      result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
+      assert.isNull(result);
     });
   });
 });
diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js b/packages/optimizely-sdk/lib/core/event_builder/index.tests.js
@@ -1,5 +1,5 @@
 /**
- * Copyright 2016-2018, Optimizely
+ * Copyright 2016-2019, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -491,15 +491,15 @@ describe('lib/core/event_builder', function() {
                 'type': 'custom',
                 'value': 'Chrome'
               }, {
-                'entity_id': '616727838',
-                'key': 'integer_key',
+                'entity_id': '808797687',
+                'key': 'valid_positive_number',
                 'type': 'custom',
-                'value': 10
+                'value': Math.pow(2, 53)
               }, {
-                'entity_id': '323434545',
-                'key': 'boolean_key',
+                'entity_id': '808797688',
+                'key': 'valid_negative_number',
                 'type': 'custom',
-                'value': false
+                'value': -Math.pow(2, 53)
               }],
               'visitor_id': 'testUser',
               'snapshots': [{
@@ -526,9 +526,10 @@ describe('lib/core/event_builder', function() {
         var eventOptions = {
           attributes: {
             'browser_type': 'Chrome',
-            'integer_key': 10,
-            'boolean_key': false,
-            'double_key': [1, 2, 3],
+            'valid_positive_number': Math.pow(2, 53),
+            'valid_negative_number': -Math.pow(2, 53),
+            'invalid_number': Math.pow(2, 53) + 2,
+            'array': [1, 2, 3],
           },
           clientEngine: 'node-sdk',
           clientVersion: packageJSON.version,
@@ -972,6 +973,81 @@ describe('lib/core/event_builder', function() {
         assert.deepEqual(actualParams, expectedParams);
       });
 
+      it('should remove invalid params from conversion event payload', function() {
+        var expectedParams = {
+          url: 'https://logx.optimizely.com/v1/events',
+          httpVerb: 'POST',
+          params: {
+            'account_id': '12001',
+            'project_id': '111001',
+            'visitors': [{
+              'visitor_id': 'testUser',
+              'attributes': [{
+                'entity_id': '111094',
+                'key': 'browser_type',
+                'type': 'custom',
+                'value': 'Chrome'
+              }, {
+                'entity_id': '808797687',
+                'key': 'valid_positive_number',
+                'type': 'custom',
+                'value': Math.pow(2, 53)
+              }, {
+                'entity_id': '808797688',
+                'key': 'valid_negative_number',
+                'type': 'custom',
+                'value': -Math.pow(2, 53)
+              }],
+              'snapshots': [{
+                'decisions': [{
+                  'variation_id': '111128',
+                  'experiment_id': '111127',
+                  'campaign_id': '4'
+                }, {
+                  'variation_id': '122228',
+                  'experiment_id': '122227',
+                  'campaign_id': '5'
+                }],
+                'events': [{
+                  'timestamp': Math.round(new Date().getTime()),
+                  'entity_id': '111100',
+                  'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c',
+                  'key': 'testEventWithMultipleExperiments'
+                }]
+              }]
+            }],
+            'revision': '42',
+            'client_name': 'node-sdk',
+            'client_version': packageJSON.version,
+            'anonymize_ip': false,
+          }
+        };
+
+        var eventOptions = {
+          clientEngine: 'node-sdk',
+          clientVersion: packageJSON.version,
+          configObj: configObj,
+          eventKey: 'testEventWithMultipleExperiments',
+          logger: mockLogger,
+          userId: 'testUser',
+          experimentsToVariationMap: {
+            '111127': '111128',
+            '122227': '122228'
+          },
+          attributes: {
+            'browser_type': 'Chrome',
+            'valid_positive_number': Math.pow(2, 53),
+            'valid_negative_number': -Math.pow(2, 53),
+            'invalid_number': -Math.pow(2, 53) - 2,
+            'array': [1, 2, 3],
+          },
+        };
+
+        var actualParams = eventBuilder.getConversionEvent(eventOptions);
+
+        assert.deepEqual(actualParams, expectedParams);
+      });
+
       describe('and event tags are passed it', function() {
         it('should create proper params for getConversionEvent with event tags', function() {
           var expectedParams = {
diff --git a/packages/optimizely-sdk/lib/core/project_config/index.tests.js b/packages/optimizely-sdk/lib/core/project_config/index.tests.js
@@ -1,5 +1,5 @@
 /**
- * Copyright 2016-2018, Optimizely
+ * Copyright 2016-2019, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -73,6 +73,10 @@ describe('lib/core/project_config', function() {
         boolean_key: testData.attributes[1],
         integer_key: testData.attributes[2],
         double_key: testData.attributes[3],
+        valid_positive_number: testData.attributes[4],
+        valid_negative_number: testData.attributes[5],
+        invalid_number: testData.attributes[6],
+        array: testData.attributes[7],
       };
 
       assert.deepEqual(configObj.attributeKeyMap, expectedAttributeKeyMap);
diff --git a/packages/optimizely-sdk/lib/tests/test_data.js b/packages/optimizely-sdk/lib/tests/test_data.js
@@ -248,6 +248,18 @@ var config = {
 		}, {
 			id: "808797686",
 			key: "double_key"
+    }, {
+			id: "808797687",
+			key: "valid_positive_number"
+    }, {
+			id: "808797688",
+			key: "valid_negative_number"
+    }, {
+			id: "808797689",
+			key: "invalid_number"
+    }, {
+			id: "808797690",
+			key: "array"
     }
   ],
   audiences: [{
diff --git a/packages/optimizely-sdk/lib/utils/attributes_validator/index.js b/packages/optimizely-sdk/lib/utils/attributes_validator/index.js
@@ -1,5 +1,5 @@
 /**
- * Copyright 2016, 2018, Optimizely
+ * Copyright 2016, 2018-2019, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,12 +20,11 @@
 
 var sprintf = require('sprintf-js').sprintf;
 var lodashForOwn = require('lodash/forOwn');
+var fns = require('../../utils/fns');
 
 var ERROR_MESSAGES = require('../enums').ERROR_MESSAGES;
 var MODULE_NAME = 'ATTRIBUTES_VALIDATOR';
 
-var VALID_ATTRIBUTE_TYPES = ['string', 'boolean', 'number'];
-
 module.exports = {
   /**
    * Validates user's provided attributes
@@ -47,6 +46,7 @@ module.exports = {
   },
 
   isAttributeValid: function(attributeKey, attributeValue) {
-    return typeof attributeKey === 'string' && VALID_ATTRIBUTE_TYPES.indexOf(typeof attributeValue) !== -1;
-  }
+    return (typeof attributeKey === 'string') && 
+    (typeof attributeValue === 'string' || typeof attributeValue === 'boolean' || fns.isFinite(attributeValue));
+  },
 };
diff --git a/packages/optimizely-sdk/lib/utils/fns/index.js b/packages/optimizely-sdk/lib/utils/fns/index.js
@@ -1,5 +1,5 @@
 /**
- * Copyright 2017, Optimizely
+ * Copyright 2017, 2019, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 var uuid = require('uuid');
+var _isFinite = require('lodash/isFinite');
+var MAX_NUMBER_LIMIT = Math.pow(2, 53);
 
 module.exports = {
   assign: require('lodash/assign'),
@@ -24,7 +26,9 @@ module.exports = {
   },
   isArray: require('lodash/isArray'),
   isEmpty: require('lodash/isEmpty'),
-  isFinite: require('lodash/isFinite'),
+  isFinite: function(number) {
+    return _isFinite(number) && Math.abs(number) <= MAX_NUMBER_LIMIT;
+  },
   keyBy: require('lodash/keyBy'),
   filter: require('lodash/filter'),
   forEach: require('lodash/forEach'),
diff --git a/packages/optimizely-sdk/lib/utils/fns/index.tests.js b/packages/optimizely-sdk/lib/utils/fns/index.tests.js
@@ -0,0 +1,41 @@
+/**
+ * Copyright 2019, Optimizely
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+var chai = require('chai');
+var assert = chai.assert;
+var fns = require('./');
+
+describe('lib/utils/fns', function() {
+  describe('APIs', function() {
+    describe('isFinite', function() {
+      it('should return false for invalid numbers', function() {
+        assert.isFalse(fns.isFinite(Infinity));
+        assert.isFalse(fns.isFinite(-Infinity));
+        assert.isFalse(fns.isFinite(NaN));
+        assert.isFalse(fns.isFinite(Math.pow(2, 53) + 2));
+        assert.isFalse(fns.isFinite(-Math.pow(2, 53) - 2));
+      });
+
+      it('should return true for valid numbers', function() {
+        assert.isTrue(fns.isFinite(0));
+        assert.isTrue(fns.isFinite(10));
+        assert.isTrue(fns.isFinite(10.5));
+        assert.isTrue(fns.isFinite(Math.pow(2, 53)));
+        assert.isTrue(fns.isFinite(-Math.pow(2, 53)));
+      });
+    });
+  });
+});
