Skip to content

Commit 02e0497

Browse files
committed
Handle multiple JSON Vulnerability Prefixes in same response
An error ("Unable to parse JSON string") occurs when the same batch response contains multiple JSON vulnerability prefixes (such as when multiple GET operations that return arrays are batched). A unit test was added to illustrate this scenario. This occurred only with multiple prefixes because string.replace was used on the entire response text, and string.replace only replaces the first occurrence of the substring. To fix this, I moved the trim function to the "httpAdapter", because that allowed finer control over when the prefix was trimmed - the prefix is now trimmed for each batch response separately and only when parsing a response with a 'json' content type. Further, trimming JSON vulnerability prefixes don't appear relevant for multifetch: the returned data is in an object form, not an array - so the vulnerability does not apply (see this for more details of how the vulnerability manifests: http://haacked.com/archive/2008/11/20/anatomy-of-a-subtle-json-vulnerability.aspx/ ).
1 parent 43d0317 commit 02e0497

File tree

3 files changed

+83
-14
lines changed

3 files changed

+83
-14
lines changed

src/services/adapters/httpAdapter.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,22 @@ function HttpBatchAdapter($document, $window, httpBatchConfig) {
177177
return boundary;
178178
}
179179

180+
/**
181+
* see https://docs.angularjs.org/api/ng/service/$http#json-vulnerability-protection
182+
* @param data
183+
* @returns {*|void|string}
184+
*/
185+
function trimJsonProtectionVulnerability(data) {
186+
return typeof (data) === 'string' ? data.replace(')]}\',\n', '') : data;
187+
}
188+
180189
function convertDataToCorrectType(contentType, dataStr) {
181190
var data = dataStr;
182191
contentType = contentType.toLowerCase();
183192

184193
if (contentType.indexOf('json') > -1) {
194+
// only remove json vulnerability prefix if we're parsing json
195+
dataStr = trimJsonProtectionVulnerability(dataStr);
185196
data = angular.fromJson(dataStr);
186197
}
187198

src/services/httpBatcher.js

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,14 @@ function addRequestFn(request) {
3434
return true;
3535
}
3636

37-
/**
38-
* see https://docs.angularjs.org/api/ng/service/$http#json-vulnerability-protection
39-
* @param data
40-
* @returns {*|void|string}
41-
*/
42-
function trimJsonProtectionVulnerability(data) {
43-
return typeof (data) === 'string' ? data.replace(')]}\',\n', '') : data;
44-
}
45-
4637
function sendFn() {
4738
var self = this,
4839
adapter = self.getAdapter(),
4940
httpBatchConfig = adapter.buildRequest(self.requests, self.config);
5041

5142
self.sendCallback();
5243
self.$injector.get('$http')(httpBatchConfig).then(function (response) {
53-
var batchResponses;
54-
55-
response.data = trimJsonProtectionVulnerability(response.data);
56-
57-
batchResponses = adapter.parseResponse(self.requests, response, self.config);
44+
var batchResponses = adapter.parseResponse(self.requests, response, self.config);
5845

5946
angular.forEach(batchResponses, function (batchResponse) {
6047
batchResponse.request.callback(

tests/services/httpBatcher.spec.js

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,77 @@
724724
$httpBackend.flush();
725725
});
726726

727+
it('should parse the response of a single batch request which contains the Angular "JSON Vulnerability Protection" prefix and multiple responses', function (done) {
728+
var batchConfig = {
729+
batchEndpointUrl: 'http://www.someservice.com/batch',
730+
batchRequestCollectionDelay: 200,
731+
minimumBatchSize: 1,
732+
adapter: defaultBatchAdapter
733+
},
734+
postData = '--31fcc127-a593-4e1d-86f3-57e45375848f\r\nContent-Type: application/http; msgtype=request\r\n\r\nGET /resource HTTP/1.1\r\nHost: www.gogle.com\r\n\r\n\r\n--31fcc127-a593-4e1d-86f3-57e45375848f' +
735+
'\r\nContent-Type: application/http; msgtype=request\r\n\r\nGET /resource-two HTTP/1.1\r\nHost: www.gogle.com\r\n\r\n\r\n--31fcc127-a593-4e1d-86f3-57e45375848f--',
736+
responseData = '--31fcc127-a593-4e1d-86f3-57e45375848f\r\nContent-Type: application/http; msgtype=response\r\n\r\n' +
737+
'HTTP/1.1 200 OK\r\nContent-Type: application/json; charset=utf-8;\r\n\r\n' +
738+
')]}\',\n' + // JSON Vulnerability Protection prefix (see https://docs.angularjs.org/api/ng/service/$http#json-vulnerability-protection )
739+
'{"results":[{"BusinessDescription":"Some text here"}],"inlineCount":35}' +
740+
'\r\n--31fcc127-a593-4e1d-86f3-57e45375848f\r\n' +
741+
'\r\nContent-Type: application/http; msgtype=response\r\n\r\n' +
742+
'HTTP/1.1 200 OK\r\nContent-Type: application/json; charset=utf-8;\r\n\r\n' +
743+
')]}\',\n' + // JSON Vulnerability Protection prefix (see https://docs.angularjs.org/api/ng/service/$http#json-vulnerability-protection )
744+
'[{"name":"Jon","id":1,"age":30},{"name":"Laurie","id":2,"age":29}]' +
745+
'\r\n--31fcc127-a593-4e1d-86f3-57e45375848f--\r\n',
746+
747+
completedFnInvocationCount = 0,
748+
749+
completedFn = function () {
750+
completedFnInvocationCount += 1;
751+
if (completedFnInvocationCount === 2) {
752+
done();
753+
}
754+
};
755+
756+
$httpBackend.expectPOST(batchConfig.batchEndpointUrl, postData).respond(200, responseData, {
757+
'content-type': 'multipart/mixed; boundary="31fcc127-a593-4e1d-86f3-57e45375848f"'
758+
}, 'OK');
759+
760+
sandbox.stub(httpBatchConfig, 'calculateBoundary').returns('31fcc127-a593-4e1d-86f3-57e45375848f');
761+
sandbox.stub(httpBatchConfig, 'getBatchConfig').returns(batchConfig);
762+
763+
httpBatcher.batchRequest({
764+
url: 'http://www.gogle.com/resource',
765+
method: 'GET',
766+
callback: function (statusCode, data) {
767+
expect(data).to.deep.equal({
768+
results: [{
769+
BusinessDescription: 'Some text here'
770+
}],
771+
inlineCount: 35
772+
});
773+
completedFn();
774+
}
775+
});
776+
777+
httpBatcher.batchRequest({
778+
url: 'http://www.gogle.com/resource-two',
779+
method: 'GET',
780+
callback: function (statusCode, data) {
781+
expect(data).to.deep.equal([{
782+
name: 'Jon',
783+
id: 1,
784+
age: 30
785+
}, {
786+
name: 'Laurie',
787+
id: 2,
788+
age: 29
789+
}]);
790+
completedFn();
791+
}
792+
});
793+
794+
$timeout.flush();
795+
$httpBackend.flush();
796+
});
797+
727798
it('should return original data for non strings when trim Angular "JSON Vulnerability Protection" prefix', function (done) {
728799
var responseData = [{
729800
"headers": {

0 commit comments

Comments
 (0)