fix (typed audience evaluation): When evaluating exact match condition, properly check type of condition value (optimizely#178)

mjc1283 · web-flow · commit 39ce59b5e7e6 · 2018-10-23T10:20:50.000-07:00
Fixes a bug in evaluation of exact match conditions. Before this change, evaluation of a condition with a wrongly-typed value property could return a boolean, when it should abort and return null. With this change, when condition.value of an exact match condition is not a string, boolean, or finite number, the condition will evaluate to null.
diff --git a/packages/optimizely-sdk/lib/core/custom_attribute_condition_evaluator/index.js b/packages/optimizely-sdk/lib/core/custom_attribute_condition_evaluator/index.js
@@ -89,7 +89,7 @@ function exactEvaluator(condition, userAttributes) {
   var userValueType = typeof userValue;
 
   if (!isValueValidForExactConditions(userValue) ||
-    !isValueValidForExactConditions(conditionValueType) ||
+    !isValueValidForExactConditions(conditionValue) ||
     conditionValueType !== userValueType) {
     return null;
   }
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
@@ -190,6 +190,17 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
         var result = customAttributeEvaluator.evaluate(exactNumberCondition, {});
         assert.isNull(result);
       });
+
+      it('should return null if the condition value is not finite', function() {
+        var invalidValueCondition = {
+          match: 'exact',
+          name: 'lasers_count',
+          type: 'custom_attribute',
+          value: Infinity,
+        };
+        var result = customAttributeEvaluator.evaluate(invalidValueCondition, { lasers_count: 9000 });
+        assert.isNull(result);
+      });
     });
 
     describe('with a boolean condition value', function() {
@@ -279,22 +290,43 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
       assert.isFalse(result);
     });
 
-    it('should return null if the user-provided value is not a number', function() {
+    it('should return null if the user-provided value is not a finite number', function() {
       var result = customAttributeEvaluator.evaluate(gtCondition, {
         meters_travelled: 'a long way',
       });
       assert.isNull(result);
 
-      var result = customAttributeEvaluator.evaluate(gtCondition, {
+      result = customAttributeEvaluator.evaluate(gtCondition, {
         meters_travelled: '1000',
       });
       assert.isNull(result);
+
+      result = customAttributeEvaluator.evaluate(gtCondition, {
+        meters_travelled: Infinity,
+      });
+      assert.isNull(result);
     });
 
     it('should return null if there is no user-provided value', function() {
       var result = customAttributeEvaluator.evaluate(gtCondition, {});
       assert.isNull(result);
     });
+
+    it('should return null if the condition value is not a finite number', function() {
+      var userAttributes = { meters_travelled: 58.4 };
+      var invalidValueCondition = {
+        match: 'gt',
+        name: 'meters_travelled',
+        type: 'custom_attribute',
+        value: Infinity,
+      };
+      var result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
+      assert.isNull(result);
+
+      invalidValueCondition.value = null;
+      result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
+      assert.isNull(result);
+    });
   });
 
   describe('less than match type', function() {
@@ -319,21 +351,42 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
       assert.isFalse(result);
     });
 
-    it('should return null if the user-provided value is not a number', function() {
+    it('should return null if the user-provided value is not a finite number', function() {
       var result = customAttributeEvaluator.evaluate(ltCondition, {
         meters_travelled: true,
       });
       assert.isNull(result);
 
-      var result = customAttributeEvaluator.evaluate(ltCondition, {
+      result = customAttributeEvaluator.evaluate(ltCondition, {
         meters_travelled: '48.2',
       });
       assert.isNull(result);
+
+      result = customAttributeEvaluator.evaluate(ltCondition, {
+        meters_travelled: Infinity,
+      });
+      assert.isNull(result);
     });
 
     it('should return null if there is no user-provided value', function() {
       var result = customAttributeEvaluator.evaluate(ltCondition, {});
       assert.isNull(result);
     });
+
+    it('should return null if the condition value is not a finite number', function() {
+      var userAttributes = { meters_travelled: 10 };
+      var invalidValueCondition = {
+        match: 'lt',
+        name: 'meters_travelled',
+        type: 'custom_attribute',
+        value: Infinity,
+      };
+      var result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
+      assert.isNull(result);
+
+      invalidValueCondition.value = {};
+      result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
+      assert.isNull(result);
+    });
   });
 });

Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ function exactEvaluator(condition, userAttributes) {`
`89`	`89`	`var userValueType = typeof userValue;`
`90`	`90`
`91`	`91`	`if (!isValueValidForExactConditions(userValue) \|\|`
`92`		`- !isValueValidForExactConditions(conditionValueType) \|\|`
	`92`	`+ !isValueValidForExactConditions(conditionValue) \|\|`
`93`	`93`	`conditionValueType !== userValueType) {`
`94`	`94`	`return null;`
`95`	`95`	`}`