Skip to content

Commit

Permalink
fix($http): parse JSON only if Content-Type header contains string 'j…
Browse files Browse the repository at this point in the history
…son'

The default behaviour breaks a wide variety of custom interpolation
markers. Even if the JSON text regexps were strengthened to closer
match the standard JSON format, it would still limit the possibilities
of different custom interpolation markers.

Instead, $http will no longer parse JSON if the response Content-Type header
does not include the term 'json', or if the $templateCache is used. For
inline templates, use the `content-type="json"` attribute to ensure that
inline JSON templates are parsed.

BREAKING CHANGE: Previously, responses would be parsed as JSON if their
content looked vaguely like JSON. Now, they are only parsed as JSON if their
Content-Type response header contains the term 'json', and if they are not
coming from the $templateCache.

Closes angular#5756
Closes angular#2973
  • Loading branch information
caitp committed Jan 13, 2014
1 parent 5aa7877 commit b7d96f6
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 13 deletions.
6 changes: 4 additions & 2 deletions src/ng/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,13 @@ function $HttpProvider() {

var defaults = this.defaults = {
// transform incoming response data
transformResponse: [function(data) {
transformResponse: [function(data, headers) {
if (isString(data)) {
// strip json vulnerability protection prefix
var contentType = isFunction(headers) && headers('Content-Type');
data = data.replace(PROTECTION_PREFIX, '');
if (JSON_START.test(data) && JSON_END.test(data))
if (JSON_START.test(data) && JSON_END.test(data) &&
(contentType && contentType.indexOf('json') >= 0))
data = fromJson(data);
}
return data;
Expand Down
40 changes: 30 additions & 10 deletions test/ng/httpSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1014,8 +1014,10 @@ describe('$http', function() {

describe('default', function() {

it('should deserialize json objects', function() {
$httpBackend.expect('GET', '/url').respond('{"foo":"bar","baz":23}');
it('should deserialize json objects with json content-type', function() {
$httpBackend.expect('GET', '/url').respond('{"foo":"bar","baz":23}', {
'Content-Type': 'application/json'
});
$http({method: 'GET', url: '/url'}).success(callback);
$httpBackend.flush();

Expand All @@ -1024,8 +1026,10 @@ describe('$http', function() {
});


it('should deserialize json arrays', function() {
$httpBackend.expect('GET', '/url').respond('[1, "abc", {"foo":"bar"}]');
it('should deserialize json arrays with json content-type', function() {
$httpBackend.expect('GET', '/url').respond('[1, "abc", {"foo":"bar"}]', {
'Content-Type': 'application/json'
});
$http({method: 'GET', url: '/url'}).success(callback);
$httpBackend.flush();

Expand All @@ -1034,8 +1038,10 @@ describe('$http', function() {
});


it('should deserialize json with security prefix', function() {
$httpBackend.expect('GET', '/url').respond(')]}\',\n[1, "abc", {"foo":"bar"}]');
it('should deserialize json with security prefix with json content-type', function() {
$httpBackend.expect('GET', '/url').respond(')]}\',\n[1, "abc", {"foo":"bar"}]', {
'Content-Type': 'application/json'
});
$http({method: 'GET', url: '/url'}).success(callback);
$httpBackend.flush();

Expand All @@ -1044,8 +1050,10 @@ describe('$http', function() {
});


it('should deserialize json with security prefix ")]}\'"', function() {
$httpBackend.expect('GET', '/url').respond(')]}\'\n\n[1, "abc", {"foo":"bar"}]');
it('should deserialize json with security prefix ")]}\'" with json content-type', function() {
$httpBackend.expect('GET', '/url').respond(')]}\'\n\n[1, "abc", {"foo":"bar"}]', {
'Content-Type': 'application/json'
});
$http({method: 'GET', url: '/url'}).success(callback);
$httpBackend.flush();

Expand All @@ -1054,8 +1062,10 @@ describe('$http', function() {
});


it('should not deserialize tpl beginning with ng expression', function() {
$httpBackend.expect('GET', '/url').respond('{{some}}');
it('should not deserialize tpl beginning with ng expression with json content-type', function() {
$httpBackend.expect('GET', '/url').respond('{{some}}', {
'Content-Type': 'application/json'
});
$http.get('/url').success(callback);
$httpBackend.flush();

Expand All @@ -1065,6 +1075,16 @@ describe('$http', function() {
});


it('should not deserialize JSON response without Content-Type header matching \'json\'', function() {
$httpBackend.expect('GET', '/url').respond('{ "foo": "bar" }');
$http.get('/url').success(callback);
$httpBackend.flush();

expect(callback).toHaveBeenCalledOnce();
expect(callback.mostRecentCall.args[0]).toBe('{ "foo": "bar" }');
});


it('should have access to response headers', function() {
$httpBackend.expect('GET', '/url').respond(200, 'response', {h1: 'header1'});
$http.get('/url', {
Expand Down
4 changes: 3 additions & 1 deletion test/ngResource/resourceSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,9 @@ describe("resource", function() {
it('should exercise full stack', function() {
var Person = $resource('/Person/:id');

$httpBackend.expect('GET', '/Person/123').respond('\n{\n"name":\n"misko"\n}\n');
$httpBackend.expect('GET', '/Person/123').respond('\n{\n"name":\n"misko"\n}\n', {
'Content-Type': 'application/json'
});
var person = Person.get({id:123});
$httpBackend.flush();
expect(person.name).toEqual('misko');
Expand Down

0 comments on commit b7d96f6

Please sign in to comment.