Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 7fe2e0e

Browse files
mfahadahmedmikeproeng37
mfahadahmed
authored and
mikeproeng37
committed
feat(attribute_value): Don't target NAN, INF, -INF and > 2^53. (optimizely#204)
1 parent d33e2cb commit 7fe2e0e

File tree

7 files changed

+179
-19
lines changed

7 files changed

+179
-19
lines changed

‎packages/optimizely-sdk/lib/core/custom_attribute_condition_evaluator/index.tests.js

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2018, Optimizely, Inc. and contributors *
2+
* Copyright 2018-2019, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -200,6 +200,10 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
200200
};
201201
var result = customAttributeEvaluator.evaluate(invalidValueCondition, { lasers_count: 9000 });
202202
assert.isNull(result);
203+
204+
invalidValueCondition.value = Math.pow(2, 53) + 2;
205+
result = customAttributeEvaluator.evaluate(invalidValueCondition, { lasers_count: 9000 });
206+
assert.isNull(result);
203207
});
204208
});
205209

@@ -305,6 +309,11 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
305309
meters_travelled: Infinity,
306310
});
307311
assert.isNull(result);
312+
313+
result = customAttributeEvaluator.evaluate(gtCondition, {
314+
meters_travelled: Math.pow(2, 53) + 2,
315+
});
316+
assert.isNull(result);
308317
});
309318

310319
it('should return null if there is no user-provided value', function() {
@@ -326,6 +335,11 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
326335
invalidValueCondition.value = null;
327336
result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
328337
assert.isNull(result);
338+
339+
invalidValueCondition.value = Math.pow(2, 53) + 2;
340+
result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
341+
assert.isNull(result);
342+
329343
});
330344
});
331345

@@ -366,6 +380,11 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
366380
meters_travelled: Infinity,
367381
});
368382
assert.isNull(result);
383+
384+
result = customAttributeEvaluator.evaluate(ltCondition, {
385+
meters_travelled: Math.pow(2, 53) + 2,
386+
});
387+
assert.isNull(result);
369388
});
370389

371390
it('should return null if there is no user-provided value', function() {
@@ -387,6 +406,10 @@ describe('lib/core/custom_attribute_condition_evaluator', function() {
387406
invalidValueCondition.value = {};
388407
result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
389408
assert.isNull(result);
409+
410+
invalidValueCondition.value = Math.pow(2, 53) + 2;
411+
result = customAttributeEvaluator.evaluate(invalidValueCondition, userAttributes);
412+
assert.isNull(result);
390413
});
391414
});
392415
});

‎packages/optimizely-sdk/lib/core/event_builder/index.tests.js

Lines changed: 86 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016-2018, Optimizely
2+
* Copyright 2016-2019, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -491,15 +491,15 @@ describe('lib/core/event_builder', function() {
491491
'type': 'custom',
492492
'value': 'Chrome'
493493
}, {
494-
'entity_id': '616727838',
495-
'key': 'integer_key',
494+
'entity_id': '808797687',
495+
'key': 'valid_positive_number',
496496
'type': 'custom',
497-
'value': 10
497+
'value': Math.pow(2, 53)
498498
}, {
499-
'entity_id': '323434545',
500-
'key': 'boolean_key',
499+
'entity_id': '808797688',
500+
'key': 'valid_negative_number',
501501
'type': 'custom',
502-
'value': false
502+
'value': -Math.pow(2, 53)
503503
}],
504504
'visitor_id': 'testUser',
505505
'snapshots': [{
@@ -526,9 +526,10 @@ describe('lib/core/event_builder', function() {
526526
var eventOptions = {
527527
attributes: {
528528
'browser_type': 'Chrome',
529-
'integer_key': 10,
530-
'boolean_key': false,
531-
'double_key': [1, 2, 3],
529+
'valid_positive_number': Math.pow(2, 53),
530+
'valid_negative_number': -Math.pow(2, 53),
531+
'invalid_number': Math.pow(2, 53) + 2,
532+
'array': [1, 2, 3],
532533
},
533534
clientEngine: 'node-sdk',
534535
clientVersion: packageJSON.version,
@@ -972,6 +973,81 @@ describe('lib/core/event_builder', function() {
972973
assert.deepEqual(actualParams, expectedParams);
973974
});
974975

976+
it('should remove invalid params from conversion event payload', function() {
977+
var expectedParams = {
978+
url: 'https://logx.optimizely.com/v1/events',
979+
httpVerb: 'POST',
980+
params: {
981+
'account_id': '12001',
982+
'project_id': '111001',
983+
'visitors': [{
984+
'visitor_id': 'testUser',
985+
'attributes': [{
986+
'entity_id': '111094',
987+
'key': 'browser_type',
988+
'type': 'custom',
989+
'value': 'Chrome'
990+
}, {
991+
'entity_id': '808797687',
992+
'key': 'valid_positive_number',
993+
'type': 'custom',
994+
'value': Math.pow(2, 53)
995+
}, {
996+
'entity_id': '808797688',
997+
'key': 'valid_negative_number',
998+
'type': 'custom',
999+
'value': -Math.pow(2, 53)
1000+
}],
1001+
'snapshots': [{
1002+
'decisions': [{
1003+
'variation_id': '111128',
1004+
'experiment_id': '111127',
1005+
'campaign_id': '4'
1006+
}, {
1007+
'variation_id': '122228',
1008+
'experiment_id': '122227',
1009+
'campaign_id': '5'
1010+
}],
1011+
'events': [{
1012+
'timestamp': Math.round(new Date().getTime()),
1013+
'entity_id': '111100',
1014+
'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c',
1015+
'key': 'testEventWithMultipleExperiments'
1016+
}]
1017+
}]
1018+
}],
1019+
'revision': '42',
1020+
'client_name': 'node-sdk',
1021+
'client_version': packageJSON.version,
1022+
'anonymize_ip': false,
1023+
}
1024+
};
1025+
1026+
var eventOptions = {
1027+
clientEngine: 'node-sdk',
1028+
clientVersion: packageJSON.version,
1029+
configObj: configObj,
1030+
eventKey: 'testEventWithMultipleExperiments',
1031+
logger: mockLogger,
1032+
userId: 'testUser',
1033+
experimentsToVariationMap: {
1034+
'111127': '111128',
1035+
'122227': '122228'
1036+
},
1037+
attributes: {
1038+
'browser_type': 'Chrome',
1039+
'valid_positive_number': Math.pow(2, 53),
1040+
'valid_negative_number': -Math.pow(2, 53),
1041+
'invalid_number': -Math.pow(2, 53) - 2,
1042+
'array': [1, 2, 3],
1043+
},
1044+
};
1045+
1046+
var actualParams = eventBuilder.getConversionEvent(eventOptions);
1047+
1048+
assert.deepEqual(actualParams, expectedParams);
1049+
});
1050+
9751051
describe('and event tags are passed it', function() {
9761052
it('should create proper params for getConversionEvent with event tags', function() {
9771053
var expectedParams = {

‎packages/optimizely-sdk/lib/core/project_config/index.tests.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016-2018, Optimizely
2+
* Copyright 2016-2019, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -73,6 +73,10 @@ describe('lib/core/project_config', function() {
7373
boolean_key: testData.attributes[1],
7474
integer_key: testData.attributes[2],
7575
double_key: testData.attributes[3],
76+
valid_positive_number: testData.attributes[4],
77+
valid_negative_number: testData.attributes[5],
78+
invalid_number: testData.attributes[6],
79+
array: testData.attributes[7],
7680
};
7781

7882
assert.deepEqual(configObj.attributeKeyMap, expectedAttributeKeyMap);

‎packages/optimizely-sdk/lib/tests/test_data.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,18 @@ var config = {
248248
}, {
249249
id: "808797686",
250250
key: "double_key"
251+
}, {
252+
id: "808797687",
253+
key: "valid_positive_number"
254+
}, {
255+
id: "808797688",
256+
key: "valid_negative_number"
257+
}, {
258+
id: "808797689",
259+
key: "invalid_number"
260+
}, {
261+
id: "808797690",
262+
key: "array"
251263
}
252264
],
253265
audiences: [{

‎packages/optimizely-sdk/lib/utils/attributes_validator/index.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016, 2018, Optimizely
2+
* Copyright 2016, 2018-2019, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -20,12 +20,11 @@
2020

2121
var sprintf = require('sprintf-js').sprintf;
2222
var lodashForOwn = require('lodash/forOwn');
23+
var fns = require('../../utils/fns');
2324

2425
var ERROR_MESSAGES = require('../enums').ERROR_MESSAGES;
2526
var MODULE_NAME = 'ATTRIBUTES_VALIDATOR';
2627

27-
var VALID_ATTRIBUTE_TYPES = ['string', 'boolean', 'number'];
28-
2928
module.exports = {
3029
/**
3130
* Validates user's provided attributes
@@ -47,6 +46,7 @@ module.exports = {
4746
},
4847

4948
isAttributeValid: function(attributeKey, attributeValue) {
50-
return typeof attributeKey === 'string' && VALID_ATTRIBUTE_TYPES.indexOf(typeof attributeValue) !== -1;
51-
}
49+
return (typeof attributeKey === 'string') &&
50+
(typeof attributeValue === 'string' || typeof attributeValue === 'boolean' || fns.isFinite(attributeValue));
51+
},
5252
};

‎packages/optimizely-sdk/lib/utils/fns/index.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2017, Optimizely
2+
* Copyright 2017, 2019, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -14,6 +14,8 @@
1414
* limitations under the License.
1515
*/
1616
var uuid = require('uuid');
17+
var _isFinite = require('lodash/isFinite');
18+
var MAX_NUMBER_LIMIT = Math.pow(2, 53);
1719

1820
module.exports = {
1921
assign: require('lodash/assign'),
@@ -24,7 +26,9 @@ module.exports = {
2426
},
2527
isArray: require('lodash/isArray'),
2628
isEmpty: require('lodash/isEmpty'),
27-
isFinite: require('lodash/isFinite'),
29+
isFinite: function(number) {
30+
return _isFinite(number) && Math.abs(number) <= MAX_NUMBER_LIMIT;
31+
},
2832
keyBy: require('lodash/keyBy'),
2933
filter: require('lodash/filter'),
3034
forEach: require('lodash/forEach'),

‎packages/optimizely-sdk/lib/utils/fns/index.tests.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* Copyright 2019, Optimizely
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
var chai = require('chai');
18+
var assert = chai.assert;
19+
var fns = require('./');
20+
21+
describe('lib/utils/fns', function() {
22+
describe('APIs', function() {
23+
describe('isFinite', function() {
24+
it('should return false for invalid numbers', function() {
25+
assert.isFalse(fns.isFinite(Infinity));
26+
assert.isFalse(fns.isFinite(-Infinity));
27+
assert.isFalse(fns.isFinite(NaN));
28+
assert.isFalse(fns.isFinite(Math.pow(2, 53) + 2));
29+
assert.isFalse(fns.isFinite(-Math.pow(2, 53) - 2));
30+
});
31+
32+
it('should return true for valid numbers', function() {
33+
assert.isTrue(fns.isFinite(0));
34+
assert.isTrue(fns.isFinite(10));
35+
assert.isTrue(fns.isFinite(10.5));
36+
assert.isTrue(fns.isFinite(Math.pow(2, 53)));
37+
assert.isTrue(fns.isFinite(-Math.pow(2, 53)));
38+
});
39+
});
40+
});
41+
});

0 commit comments

Comments
 (0)