From 3b6f3640a292c8dd5eb551b814fa9b9ed9ce13cc Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Sun, 7 Dec 2014 14:53:08 +0200 Subject: [PATCH] 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 --- src/ng/http.js | 28 ++++++++++++++++++++-------- test/ng/httpSpec.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index e7a84024295c..5a5dcfca72e1 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -2,23 +2,35 @@ 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)) { + // Parse JSON + 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 * diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index 691542956ad5..27d79684ef9a 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -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"]'); + } + ); });