Skip to content
Permalink
Browse files

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 #10349
Closes #10357
  • Loading branch information
gkalpak authored and pkozlowski-opensource committed Dec 7, 2014
1 parent 6617b42 commit b9bdbe615cc4070d2233ff06830a4c6fb1217cda
Showing with 53 additions and 8 deletions.
  1. +19 −8 src/ng/http.js
  2. +34 −0 test/ng/httpSpec.js
@@ -2,23 +2,34 @@

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

function defaultHttpResponseTransform(data, headers) {
if (isString(data)) {
// strip json vulnerability protection prefix
data = data.replace(JSON_PROTECTION_PREFIX, '');
var contentType = headers('Content-Type');
if ((contentType && contentType.indexOf(APPLICATION_JSON) === 0 && data.trim()) ||
(JSON_START.test(data) && JSON_END.test(data))) {
data = fromJson(data);
// Strip json vulnerability protection prefix and trim whitespace
var tempData = data.replace(JSON_PROTECTION_PREFIX, '').trim();

if (tempData) {
var contentType = headers('Content-Type');
if ((contentType && (contentType.indexOf(APPLICATION_JSON) === 0)) || isJsonLike(tempData)) {
data = fromJson(tempData);
}
}
}

return data;
}

function isJsonLike(str) {
var jsonStart = str.match(JSON_START);
return jsonStart && JSON_ENDS[jsonStart[0]].test(str);
}

/**
* Parse headers into key value object
*
@@ -1055,6 +1055,16 @@ describe('$http', function() {
});


it('should ignore leading/trailing whitespace', function() {
$httpBackend.expect('GET', '/url').respond(' \n {"foo":"bar","baz":23} \r\n \n ');
$http({method: 'GET', url: '/url'}).success(callback);
$httpBackend.flush();

expect(callback).toHaveBeenCalledOnce();
expect(callback.mostRecentCall.args[0]).toEqual({foo: 'bar', baz: 23});
});


it('should deserialize json numbers when response header contains application/json',
function() {
$httpBackend.expect('GET', '/url').respond('123', {'Content-Type': 'application/json'});
@@ -1141,6 +1151,16 @@ describe('$http', function() {
});


it('should retain security prefix if response is not json', function() {
$httpBackend.expect('GET', '/url').respond(')]}\',\n This is not JSON !');
$http({method: 'GET', url: '/url'}).success(callback);
$httpBackend.flush();

expect(callback).toHaveBeenCalledOnce();
expect(callback.mostRecentCall.args[0]).toEqual(')]}\',\n This is not JSON !');
});


it('should not attempt to deserialize json when HEAD request', function() {
//per http spec for Content-Type, HEAD request should return a Content-Type header
//set to what the content type would have been if a get was sent
@@ -1182,6 +1202,20 @@ describe('$http', function() {
expect(callback).toHaveBeenCalledOnce();
expect(callback.mostRecentCall.args[0]).toEqual('{{some}}');
});

it('should not deserialize json when the opening and closing brackets do not match',
function() {
$httpBackend.expect('GET', '/url1').respond('[Code](url): function() {}');
$httpBackend.expect('GET', '/url2').respond('{"is": "not"} ["json"]');
$http.get('/url1').success(callback);
$http.get('/url2').success(callback);
$httpBackend.flush();

expect(callback.calls.length).toBe(2);
expect(callback.calls[0].args[0]).toEqual('[Code](url): function() {}');
expect(callback.calls[1].args[0]).toEqual('{"is": "not"} ["json"]');
}
);
});


0 comments on commit b9bdbe6

Please sign in to comment.
You can’t perform that action at this time.