Skip to content

Commit 3b6f364

Browse files
committed
fix($http): only parse as JSON when opening/closing brackets match
Previously, due to weak JSON-detecting RegExp, string like `[...}` and `{...]` would be considered JSON (even if they obviously aren't) and an expection would be thrown while trying to parse them. This commit makes sure the opening and closing brackets match. This doesn't completely eliminate false positives (e.g. `[]{}[]`), but does help reduce them. Closes angular#10349
1 parent 6617b42 commit 3b6f364

File tree

2 files changed

+54
-8
lines changed

2 files changed

+54
-8
lines changed

src/ng/http.js

+20-8
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,35 @@
22

33
var APPLICATION_JSON = 'application/json';
44
var CONTENT_TYPE_APPLICATION_JSON = {'Content-Type': APPLICATION_JSON + ';charset=utf-8'};
5-
var JSON_START = /^\s*(\[|\{[^\{])/;
6-
var JSON_END = /[\}\]]\s*$/;
5+
var JSON_START = /^\[|^\{(?!\{)/;
6+
var JSON_ENDS = {
7+
'[': /]$/,
8+
'{': /}$/
9+
};
710
var JSON_PROTECTION_PREFIX = /^\)\]\}',?\n/;
811

912
function defaultHttpResponseTransform(data, headers) {
1013
if (isString(data)) {
11-
// strip json vulnerability protection prefix
12-
data = data.replace(JSON_PROTECTION_PREFIX, '');
13-
var contentType = headers('Content-Type');
14-
if ((contentType && contentType.indexOf(APPLICATION_JSON) === 0 && data.trim()) ||
15-
(JSON_START.test(data) && JSON_END.test(data))) {
16-
data = fromJson(data);
14+
// Strip json vulnerability protection prefix and trim whitespace
15+
var tempData = data.replace(JSON_PROTECTION_PREFIX, '').trim();
16+
17+
if (tempData) {
18+
var contentType = headers('Content-Type');
19+
if ((contentType && (contentType.indexOf(APPLICATION_JSON) === 0)) || isJsonLike(tempData)) {
20+
// Parse JSON
21+
data = fromJson(tempData);
22+
}
1723
}
1824
}
25+
1926
return data;
2027
}
2128

29+
function isJsonLike(str) {
30+
var jsonStart = str.match(JSON_START);
31+
return jsonStart && JSON_ENDS[jsonStart[0]].test(str);
32+
}
33+
2234
/**
2335
* Parse headers into key value object
2436
*

test/ng/httpSpec.js

+34
Original file line numberDiff line numberDiff line change
@@ -1055,6 +1055,16 @@ describe('$http', function() {
10551055
});
10561056

10571057

1058+
it('should ignore leading/trailing whitespace', function() {
1059+
$httpBackend.expect('GET', '/url').respond(' \n {"foo":"bar","baz":23} \r\n \n ');
1060+
$http({method: 'GET', url: '/url'}).success(callback);
1061+
$httpBackend.flush();
1062+
1063+
expect(callback).toHaveBeenCalledOnce();
1064+
expect(callback.mostRecentCall.args[0]).toEqual({foo: 'bar', baz: 23});
1065+
});
1066+
1067+
10581068
it('should deserialize json numbers when response header contains application/json',
10591069
function() {
10601070
$httpBackend.expect('GET', '/url').respond('123', {'Content-Type': 'application/json'});
@@ -1141,6 +1151,16 @@ describe('$http', function() {
11411151
});
11421152

11431153

1154+
it('should retain security prefix if response is not json', function() {
1155+
$httpBackend.expect('GET', '/url').respond(')]}\',\n This is not JSON !');
1156+
$http({method: 'GET', url: '/url'}).success(callback);
1157+
$httpBackend.flush();
1158+
1159+
expect(callback).toHaveBeenCalledOnce();
1160+
expect(callback.mostRecentCall.args[0]).toEqual(')]}\',\n This is not JSON !');
1161+
});
1162+
1163+
11441164
it('should not attempt to deserialize json when HEAD request', function() {
11451165
//per http spec for Content-Type, HEAD request should return a Content-Type header
11461166
//set to what the content type would have been if a get was sent
@@ -1182,6 +1202,20 @@ describe('$http', function() {
11821202
expect(callback).toHaveBeenCalledOnce();
11831203
expect(callback.mostRecentCall.args[0]).toEqual('{{some}}');
11841204
});
1205+
1206+
it('should not deserialize json when the opening and closing brackets do not match',
1207+
function() {
1208+
$httpBackend.expect('GET', '/url1').respond('[Code](url): function() {}');
1209+
$httpBackend.expect('GET', '/url2').respond('{"is": "not"} ["json"]');
1210+
$http.get('/url1').success(callback);
1211+
$http.get('/url2').success(callback);
1212+
$httpBackend.flush();
1213+
1214+
expect(callback.calls.length).toBe(2);
1215+
expect(callback.calls[0].args[0]).toEqual('[Code](url): function() {}');
1216+
expect(callback.calls[1].args[0]).toEqual('{"is": "not"} ["json"]');
1217+
}
1218+
);
11851219
});
11861220

11871221

0 commit comments

Comments
 (0)