Skip to content

Commit 449849c

Browse files
author
Tyler Brandt
authored
Fix vulnerabilities by removing request dependency (#98)
Summary: [`srcclr`](https://optimizely.sourceclear.io/teams/3OOu2k/scans/3772139?login=saml) flags `request` and some of its transitive dependencies as vulnerable. Since we are only barely using it, we can switch to the native node modules `http`/`https` instead. The result is a [clean run](https://optimizely.sourceclear.io/teams/3OOu2k/scans/3772270?login=saml). ``` $ srcclr scan . SourceClear scanning engine ready Running the NPM scanner Scanning completed Found 12997 lines of code Processing results... Processing results complete Summary Report Scan ID 614c7376-d1ae-4d77-ad5f-3078f6a17917 Scan Date & Time May 16 2018 04:26PM PDT Account type PRO Scan engine 2.14.7 (latest 2.14.7) Analysis time 12 seconds User tbrandt Project /Users/tbrandt/optly/Projects/javascript-sdk/packages/optimizely-sdk Package Manager(s) NPM Open-Source Libraries Total Libraries 5 Direct Libraries 5 Transitive Libraries 0 Vulnerable Libraries 0 Third Party Code 77.4% Security With Vulnerable Methods 0 High Risk Vulnerabilities 0 Medium Risk Vulnerabilities 0 Low Risk Vulnerabilities 0 Licenses Unique Library Licenses 3 Libraries Using GPL 0 Libraries With No License 1 Libraries With Multiple Licenses 1 Full Report Details https://optimizely.sourceclear.io/teams/3OOu2k/scans/3772270?login=saml ``` Also add srcclr.yml with `scope: production` so we can scan more easily in the correct way. Test Plan: Existing automated tests Reviewers: matt.carroll, ola.nordstrom, michael.hood, ali, greeshma Reviewed By: ali, greeshma JIRA Issues: OASIS-2776 Differential Revision: https://phabricator.optimizely.com/D19829
1 parent 149fada commit 449849c

File tree

5 files changed

+66
-28
lines changed

5 files changed

+66
-28
lines changed

packages/optimizely-sdk/lib/optimizely/index.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ Optimizely.prototype.getForcedVariation = function(experimentKey, userId) {
362362
Optimizely.prototype.__validateInputs = function(stringInputs, userAttributes, eventTags) {
363363
try {
364364
var inputKeys = Object.keys(stringInputs);
365-
for (var index=0; index < inputKeys.length; index++) {
365+
for (var index = 0; index < inputKeys.length; index++) {
366366
var key = inputKeys[index];
367367
if (!stringValidator.validate(stringInputs[key])) {
368368
throw new Error(sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, MODULE_NAME, key));
@@ -449,12 +449,12 @@ Optimizely.prototype.__notActivatingExperiment = function(experimentKey, userId)
449449
Optimizely.prototype.__dispatchEvent = function (eventToDispatch, callback) {
450450
var eventDispatcherResponse = this.eventDispatcher.dispatchEvent(eventToDispatch, callback);
451451
//checking that response value is a promise, not a request object
452-
if (typeof eventDispatcherResponse == "object" && !eventDispatcherResponse.hasOwnProperty('uri')) {
452+
if (!fns.isEmpty(eventDispatcherResponse) && typeof eventDispatcherResponse.then === 'function') {
453453
eventDispatcherResponse.then(function() {
454454
callback();
455455
});
456456
}
457-
}
457+
};
458458

459459
/**
460460
* Filters out attributes/eventTags with null or undefined values
@@ -464,11 +464,11 @@ Optimizely.prototype.__dispatchEvent = function (eventToDispatch, callback) {
464464
Optimizely.prototype.__filterEmptyValues = function (map) {
465465
for (var key in map) {
466466
if (map.hasOwnProperty(key) && (map[key] === null || map[key] === undefined)) {
467-
delete map[key]
467+
delete map[key];
468468
}
469469
}
470470
return map;
471-
}
471+
};
472472

473473
/**
474474
* Returns true if the feature is enabled for the given user.
Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016-2017, Optimizely
2+
* Copyright 2016-2018, 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.
@@ -13,35 +13,54 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
var request = require('request');
17-
18-
var POST_HEADERS = {'content-type': 'application/json'};
16+
var http = require('http');
17+
var https = require('https');
18+
var url = require('url');
1919

2020
module.exports = {
2121
/**
2222
* Dispatch an HTTP request to the given url and the specified options
2323
* @param {Object} eventObj Event object containing
2424
* @param {string} eventObj.url the url to make the request to
25-
* @param {Object} eventObj.params parameters to pass to the request
26-
* @param {string} eventObj.httpVerb the HTTP request method type
25+
* @param {Object} eventObj.params parameters to pass to the request (i.e. in the POST body)
26+
* @param {string} eventObj.httpVerb the HTTP request method type. only POST is supported.
2727
* @param {function} callback callback to execute
28-
* @return {Promise<Object>} the payload from the request
28+
* @return {ClientRequest|undefined} ClientRequest object which made the request, or undefined if no request was made (error)
2929
*/
3030
dispatchEvent: function(eventObj, callback) {
31+
// Non-POST requests not supported
32+
if (eventObj.httpVerb !== 'POST') {
33+
callback(new Error('httpVerb not supported: ' + eventObj.httpVerb));
34+
return;
35+
}
36+
37+
var parsedUrl = url.parse(eventObj.url);
38+
var path = parsedUrl.path;
39+
if (parsedUrl.query) {
40+
path += '?' + parsedUrl.query;
41+
}
42+
43+
var dataString = JSON.stringify(eventObj.params);
44+
3145
var requestOptions = {
32-
uri: eventObj.url,
33-
body: eventObj.params,
34-
headers: POST_HEADERS,
35-
method: eventObj.httpVerb,
36-
json: true,
46+
host: parsedUrl.host,
47+
path: parsedUrl.path,
48+
method: 'POST',
49+
headers: {
50+
'content-type': 'application/json',
51+
'content-length': dataString.length.toString(),
52+
}
3753
};
3854

3955
var requestCallback = function(error, response, body) {
4056
if (response && response.statusCode && response.statusCode >= 200 && response.statusCode < 400 && callback && typeof callback == 'function') {
4157
callback();
4258
}
43-
}
59+
};
4460

45-
return request(requestOptions, requestCallback);
61+
var req = (parsedUrl.protocol === 'http:' ? http : https).request(requestOptions, requestCallback);
62+
req.write(dataString);
63+
req.end();
64+
return req;
4665
}
4766
};

packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016-2017, Optimizely
2+
* Copyright 2016-2018, 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.
@@ -24,7 +24,7 @@ describe('lib/plugins/event_dispatcher/node', function() {
2424
describe('dispatchEvent', function() {
2525
var stubCallback = {
2626
callback: function() {}
27-
}
27+
};
2828

2929
beforeEach(function() {
3030
sinon.stub(stubCallback, 'callback');
@@ -43,7 +43,7 @@ describe('lib/plugins/event_dispatcher/node', function() {
4343
it('should send a POST request with the specified params', function(done) {
4444
var eventObj = {
4545
url: 'https://cdn.com/event',
46-
body: {
46+
params: {
4747
id: 123,
4848
},
4949
httpVerb: 'POST',
@@ -55,14 +55,14 @@ describe('lib/plugins/event_dispatcher/node', function() {
5555
done();
5656
})
5757
.on('error', function(error) {
58-
assert.fail('status code okay', 'status code not okay', "")
59-
})
58+
assert.fail('status code okay', 'status code not okay', '');
59+
});
6060
});
6161

6262
it('should execute the callback passed to event dispatcher', function(done) {
6363
var eventObj = {
6464
url: 'https://cdn.com/event',
65-
body: {
65+
params: {
6666
id: 123,
6767
},
6868
httpVerb: 'POST',
@@ -74,10 +74,29 @@ describe('lib/plugins/event_dispatcher/node', function() {
7474
sinon.assert.calledOnce(stubCallback.callback);
7575
})
7676
.on('error', function(error) {
77-
assert.fail('status code okay', 'status code not okay', "")
78-
})
77+
assert.fail('status code okay', 'status code not okay', '');
78+
});
7979
});
8080

81+
it('rejects GET httpVerb', function(done) {
82+
var eventObj = {
83+
url: 'https://cdn.com/event',
84+
params: {
85+
id: 123,
86+
},
87+
httpVerb: 'GET',
88+
};
89+
90+
var callback = function(err) {
91+
if (err) {
92+
done();
93+
} else {
94+
done(new Error('expected error not thrown'));
95+
}
96+
};
97+
98+
eventDispatcher.dispatchEvent(eventObj, callback);
99+
});
81100
});
82101
});
83102
});

packages/optimizely-sdk/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
"json-schema": "^0.2.3",
3636
"lodash": "^4.0.0",
3737
"murmurhash": "0.0.2",
38-
"request": "~2.83.0",
3938
"sprintf": "^0.1.5",
4039
"uuid": "^3.0.1"
4140
},

packages/optimizely-sdk/srcclr.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
scope: production

0 commit comments

Comments
 (0)