Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($templateRequest): ignore JSON Content-Type header and content #9619

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/.jshintrc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@
"NODE_TYPE_DOCUMENT": false,
"NODE_TYPE_DOCUMENT_FRAGMENT": false,

"httpResponseTransform": false,

/* filters.js */
"getFirstThursdayOfYear": false,

Expand Down
38 changes: 20 additions & 18 deletions src/ng/http.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
'use strict';

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_PROTECTION_PREFIX = /^\)\]\}',?\n/;

function httpResponseTransform(data, headers) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rename to defaultHttpResponseTransform

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) ||
(JSON_START.test(data) && JSON_END.test(data))) {
data = fromJson(data);
}
}
return data;
}

/**
* Parse headers into key value object
*
Expand Down Expand Up @@ -86,12 +105,6 @@ function isSuccess(status) {
* Use `$httpProvider` to change the default behavior of the {@link ng.$http $http} service.
* */
function $HttpProvider() {
var JSON_START = /^\s*(\[|\{[^\{])/,
JSON_END = /[\}\]]\s*$/,
PROTECTION_PREFIX = /^\)\]\}',?\n/,
APPLICATION_JSON = 'application/json',
CONTENT_TYPE_APPLICATION_JSON = {'Content-Type': APPLICATION_JSON + ';charset=utf-8'};

/**
* @ngdoc property
* @name $httpProvider#defaults
Expand All @@ -115,18 +128,7 @@ function $HttpProvider() {
**/
var defaults = this.defaults = {
// transform incoming response data
transformResponse: [function defaultHttpResponseTransform(data, headers) {
if (isString(data)) {
// strip json vulnerability protection prefix
data = data.replace(PROTECTION_PREFIX, '');
var contentType = headers('Content-Type');
if ((contentType && contentType.indexOf(APPLICATION_JSON) === 0) ||
(JSON_START.test(data) && JSON_END.test(data))) {
data = fromJson(data);
}
}
return data;
}],
transformResponse: [httpResponseTransform],

// transform outgoing request data
transformRequest: [function(d) {
Expand Down
18 changes: 17 additions & 1 deletion src/ng/templateRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,23 @@ function $TemplateRequestProvider() {
var self = handleRequestFn;
self.totalPendingRequests++;

return $http.get(tpl, { cache : $templateCache })
var transformResponse = $http.defaults && $http.defaults.transformResponse;
var idx;

if (isArray(transformResponse) &&
(idx = transformResponse.indexOf(httpResponseTransform)) >= 0) {
transformResponse = copy(transformResponse, []);
transformResponse.splice(idx, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just slice it (instead of copy + splice) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slice would only work if the item to be removed is at the very beginning or very end of the array

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no guarantee of this, even if it is the common case

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you are doing 3 "heavy" array operations (indexof, copy, splice), it might be better to just have one for loop to manually copy stuff over except for the default transform

} else if (transformResponse === httpResponseTransform) {
transformResponse = null;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to avoid breaking change


var httpOptions = {
cache: $templateCache,
transformResponse: transformResponse
};

return $http.get(tpl, httpOptions)
.then(function(response) {
var html = response.data;
if(!html || html.length === 0) {
Expand Down
44 changes: 44 additions & 0 deletions test/ng/templateRequestSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,48 @@ describe('$templateRequest', function() {
expect($templateRequest.totalPendingRequests).toBe(0);
}));

it('should not try to parse a response as JSON',
inject(function($templateRequest, $httpBackend) {
var spy = jasmine.createSpy('success');
$httpBackend.expectGET('a.html').respond('{{text}}', {
'Content-Type': 'application/json'
});
$templateRequest('a.html').then(spy);
$httpBackend.flush();
expect(spy).toHaveBeenCalledOnceWith('{{text}}');
}));

it('should use custom response transformers (array)', function() {
module(function($httpProvider) {
$httpProvider.defaults.transformResponse.push(function(data) {
return data + '!!';
});
});
inject(function($templateRequest, $httpBackend) {
var spy = jasmine.createSpy('success');
$httpBackend.expectGET('a.html').respond('{{text}}', {
'Content-Type': 'application/json'
});
$templateRequest('a.html').then(spy);
$httpBackend.flush();
expect(spy).toHaveBeenCalledOnceWith('{{text}}!!');
});
});

it('should use custom response transformers (function)', function() {
module(function($httpProvider) {
$httpProvider.defaults.transformResponse = function(data) {
return data + '!!';
};
});
inject(function($templateRequest, $httpBackend) {
var spy = jasmine.createSpy('success');
$httpBackend.expectGET('a.html').respond('{{text}}', {
'Content-Type': 'application/json'
});
$templateRequest('a.html').then(spy);
$httpBackend.flush();
expect(spy).toHaveBeenCalledOnceWith('{{text}}!!');
});
});
});