diff --git a/src/ng/http.js b/src/ng/http.js index 5bf415714a8f..681065335b8d 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -1099,7 +1099,7 @@ function $HttpProvider() { * * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested; * or an object created by a call to `$sce.trustAsResourceUrl(url)`. - * @param {Object=} config Optional configuration object + * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage * @returns {HttpPromise} Future object */ @@ -1112,7 +1112,7 @@ function $HttpProvider() { * * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested; * or an object created by a call to `$sce.trustAsResourceUrl(url)`. - * @param {Object=} config Optional configuration object + * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage * @returns {HttpPromise} Future object */ @@ -1125,7 +1125,7 @@ function $HttpProvider() { * * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested; * or an object created by a call to `$sce.trustAsResourceUrl(url)`. - * @param {Object=} config Optional configuration object + * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage * @returns {HttpPromise} Future object */ @@ -1142,6 +1142,10 @@ function $HttpProvider() { * {@link $sceDelegateProvider#resourceUrlWhitelist `$sceDelegateProvider.resourceUrlWhitelist`} or * by explicitly trusting the URL via {@link $sce#trustAsResourceUrl `$sce.trustAsResourceUrl(url)`}. * + * You should avoid generating the URL for the JSONP request from user provided data. + * Provide additional query parameters via `params` property of the `config` parameter, rather than + * modifying the URL itself. + * * JSONP requests must specify a callback to be used in the response from the server. This callback * is passed as a query parameter in the request. You must specify the name of this parameter by * setting the `jsonpCallbackParam` property on the request config object. @@ -1163,7 +1167,7 @@ function $HttpProvider() { * * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested; * or an object created by a call to `$sce.trustAsResourceUrl(url)`. - * @param {Object=} config Optional configuration object + * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage * @returns {HttpPromise} Future object */ createShortMethods('get', 'delete', 'head', 'jsonp'); @@ -1177,7 +1181,7 @@ function $HttpProvider() { * * @param {string} url Relative or absolute URL specifying the destination of the request * @param {*} data Request content - * @param {Object=} config Optional configuration object + * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage * @returns {HttpPromise} Future object */ @@ -1190,7 +1194,7 @@ function $HttpProvider() { * * @param {string} url Relative or absolute URL specifying the destination of the request * @param {*} data Request content - * @param {Object=} config Optional configuration object + * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage * @returns {HttpPromise} Future object */ @@ -1203,7 +1207,7 @@ function $HttpProvider() { * * @param {string} url Relative or absolute URL specifying the destination of the request * @param {*} data Request content - * @param {Object=} config Optional configuration object + * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage * @returns {HttpPromise} Future object */ createShortMethodsWithData('post', 'put', 'patch'); @@ -1417,20 +1421,26 @@ function $HttpProvider() { return url; } - function sanitizeJsonpCallbackParam(url, key) { - if (/[&?][^=]+=JSON_CALLBACK/.test(url)) { - // Throw if the url already contains a reference to JSON_CALLBACK - throw $httpMinErr('badjsonp', 'Illegal use of JSON_CALLBACK in url, "{0}"', url); - } - - var callbackParamRegex = new RegExp('[&?]' + key + '='); - if (callbackParamRegex.test(url)) { - // Throw if the callback param was already provided - throw $httpMinErr('badjsonp', 'Illegal use of callback param, "{0}", in url, "{1}"', key, url); + function sanitizeJsonpCallbackParam(url, cbKey) { + var parts = url.split('?'); + if (parts.length > 2) { + // Throw if the url contains more than one `?` query indicator + throw $httpMinErr('badjsonp', 'Illegal use more than one "?", in url, "{1}"', url); } + var params = parseKeyValue(parts[1]); + forEach(params, function(value, key) { + if (value === 'JSON_CALLBACK') { + // Throw if the url already contains a reference to JSON_CALLBACK + throw $httpMinErr('badjsonp', 'Illegal use of JSON_CALLBACK in url, "{0}"', url); + } + if (key === cbKey) { + // Throw if the callback param was already provided + throw $httpMinErr('badjsonp', 'Illegal use of callback param, "{0}", in url, "{1}"', cbKey, url); + } + }); // Add in the JSON_CALLBACK callback param value - url += ((url.indexOf('?') === -1) ? '?' : '&') + key + '=JSON_CALLBACK'; + url += ((url.indexOf('?') === -1) ? '?' : '&') + cbKey + '=JSON_CALLBACK'; return url; } diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index f7b25ccb12c9..59429ba26612 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -979,6 +979,14 @@ describe('$http', function() { $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), params: {a: 'b'}}); }); + it('should error if the URL contains more than one `?` query indicator', function() { + var error; + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?a=b?c=d')}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error).toEqualMinErr('$http', 'badjsonp'); + }); + it('should error if the URL contains a JSON_CALLBACK parameter', function() { var error; $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?callback=JSON_CALLBACK')}) @@ -986,11 +994,23 @@ describe('$http', function() { $rootScope.$digest(); expect(error).toEqualMinErr('$http', 'badjsonp'); + error = undefined; + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?callback=JSON_C%41LLBACK')}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error).toEqualMinErr('$http', 'badjsonp'); + error = undefined; $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?other=JSON_CALLBACK')}) .catch(function(e) { error = e; }); $rootScope.$digest(); expect(error).toEqualMinErr('$http', 'badjsonp'); + + error = undefined; + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?other=JSON_C%41LLBACK')}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error).toEqualMinErr('$http', 'badjsonp'); }); it('should error if a param contains a JSON_CALLBACK value', function() { @@ -1007,6 +1027,23 @@ describe('$http', function() { expect(error).toEqualMinErr('$http', 'badjsonp'); }); + it('should allow encoded params that look like they contain the value JSON_CALLBACK or the configured callback key', function() { + var error; + error = undefined; + $httpBackend.expect('JSONP', 'http://example.org/path?other=JSON_C%2541LLBACK&callback=JSON_CALLBACK').respond(''); + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {other: 'JSON_C%41LLBACK'}}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error).toBeUndefined(); + + error = undefined; + $httpBackend.expect('JSONP', 'http://example.org/path?c%2561llback=evilThing&callback=JSON_CALLBACK').respond(''); + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {'c%61llback': 'evilThing'}}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error).toBeUndefined(); + }); + it('should error if there is already a param matching the jsonpCallbackParam key', function() { var error; $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {callback: 'evilThing'}}) @@ -1014,11 +1051,23 @@ describe('$http', function() { $rootScope.$digest(); expect(error).toEqualMinErr('$http', 'badjsonp'); + error = undefined; + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?c%61llback=evilThing')}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error).toEqualMinErr('$http', 'badjsonp'); + error = undefined; $http({ method: 'JSONP', jsonpCallbackParam: 'cb', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {cb: 'evilThing'}}) .catch(function(e) { error = e; }); $rootScope.$digest(); expect(error).toEqualMinErr('$http', 'badjsonp'); + + error = undefined; + $http({ method: 'JSONP', jsonpCallbackParam: 'cb', url: $sce.trustAsResourceUrl('http://example.org/path?c%62=evilThing')}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error).toEqualMinErr('$http', 'badjsonp'); }); });