From 9d08b33a0dd9d4a45dda512c95db12555baa9d17 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Mon, 19 Sep 2016 13:54:22 +0100 Subject: [PATCH 1/3] test($route): ensure mock $sce delegate is implemented correctly The mock calls to `valueOf(v)` and `getTrustedResourceUrl(v)` were not dealing with the case where `v` was null. --- test/ngRoute/routeSpec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/ngRoute/routeSpec.js b/test/ngRoute/routeSpec.js index 9884a199f1d2..75e670b66243 100644 --- a/test/ngRoute/routeSpec.js +++ b/test/ngRoute/routeSpec.js @@ -1028,9 +1028,10 @@ describe('$route', function() { $routeProvider = _$routeProvider_; $provide.decorator('$sce', function($delegate) { + function getVal(v) { return v.getVal ? v.getVal() : v; } $delegate.trustAsResourceUrl = function(url) { return new MySafeResourceUrl(url); }; - $delegate.getTrustedResourceUrl = function(v) { return v.getVal(); }; - $delegate.valueOf = function(v) { return v.getVal(); }; + $delegate.getTrustedResourceUrl = function(v) { return getVal(v); }; + $delegate.valueOf = function(v) { return getVal(v); }; return $delegate; }); }); From 6476af83cd0418c84e034a955b12a842794385c4 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 20 Sep 2016 18:41:22 +0100 Subject: [PATCH 2/3] feat($http): JSONP requests now require a trusted resource URL The $http service will reject JSONP requests that are not trusted by `$sce` as "ResourceUrl". This change makes is easier for developers to see clearly where in their code they are making JSONP calls that may be to untrusted endpoings and forces them to think about how these URLs are generated. Be aware that this commit does not put any constraint on the parameters that will be appended to the URL. Developers should be mindful of what parameters can be attached and how they are generated. Closes #11352 BREAKING CHANGE All JSONP requests now require the URL to be trusted as resource URLs. There are two approaches to trust a URL: **Whitelisting with the `$sceDelegateProvider.resourceUrlWhitelist()` method.** You configure this list in a module configuration block: ``` appModule.config(['$sceDelegateProvider', function($sceDelegateProvider) { $sceDelegateProvider.resourceUrlWhiteList([ // Allow same origin resource loads. 'self', // Allow JSONP calls that match this pattern 'https://some.dataserver.com/**.jsonp?**` ]); }]); ``` **Explicitly trusting the URL via the `$sce.trustAsResourceUrl(url)` method** You can pass a trusted object instead of a string as a URL to the `$http` service: ``` var promise = $http.jsonp($sce.trustAsResourceUrl(url)); ``` --- docs/content/error/$http/badreq.ngdoc | 6 ++- src/ng/http.js | 54 ++++++++++++++++------ test/ng/httpSpec.js | 66 ++++++++++++++++++++++----- 3 files changed, 101 insertions(+), 25 deletions(-) diff --git a/docs/content/error/$http/badreq.ngdoc b/docs/content/error/$http/badreq.ngdoc index ea2f5006361c..81ea6349ecfe 100644 --- a/docs/content/error/$http/badreq.ngdoc +++ b/docs/content/error/$http/badreq.ngdoc @@ -3,7 +3,11 @@ @fullName Bad Request Configuration @description -This error occurs when the request configuration parameter passed to the {@link ng.$http `$http`} service is not an object.  `$http` expects a single parameter, the request configuration object, but received a parameter that was not an object.  The error message should provide additional context such as the actual value of the parameter that was received.  If you passed a string parameter, perhaps you meant to call one of the shorthand methods on `$http` such as `$http.get(…)`, etc. +This error occurs when the request configuration parameter passed to the {@link ng.$http `$http`} service is not a valid object. +`$http` expects a single parameter, the request configuration object, but received a parameter that was not an object or did not contain valid properties. + +The error message should provide additional context such as the actual value of the parameter that was received. +If you passed a string parameter, perhaps you meant to call one of the shorthand methods on `$http` such as `$http.get(…)`, etc. To resolve this error, make sure you pass a valid request configuration object to `$http`. diff --git a/src/ng/http.js b/src/ng/http.js index cf787bf58e89..a7b99d809028 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -379,8 +379,8 @@ function $HttpProvider() { **/ var interceptorFactories = this.interceptors = []; - this.$get = ['$browser', '$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector', - function($browser, $httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) { + this.$get = ['$browser', '$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector', '$sce', + function($browser, $httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector, $sce) { var defaultCache = $cacheFactory('$http'); @@ -802,7 +802,8 @@ function $HttpProvider() { * processed. The object has following properties: * * - **method** – `{string}` – HTTP method (e.g. 'GET', 'POST', etc) - * - **url** – `{string}` – Absolute or relative URL of the resource that is being requested. + * - **url** – `{string|TrustedObject}` – Absolute or relative URL of the resource that is being requested; + * or an object created by a call to `$sce.trustAsResourceUrl(url)`. * - **params** – `{Object.}` – Map of strings or objects which will be serialized * with the `paramSerializer` and appended as GET parameters. * - **data** – `{string|Object}` – Data to be sent as the request message data. @@ -881,6 +882,13 @@ function $HttpProvider() { angular.module('httpExample', []) + .config(['$sceDelegateProvider', function($sceDelegateProvider) { + // We must whitelist the JSONP endpoint that we are using to show that we trust it + $sceDelegateProvider.resourceUrlWhitelist([ + 'self', + 'https://angularjs.org/**' + ]); + }]) .controller('FetchController', ['$scope', '$http', '$templateCache', function($scope, $http, $templateCache) { $scope.method = 'GET'; @@ -948,8 +956,8 @@ function $HttpProvider() { throw minErr('$http')('badreq', 'Http request configuration must be an object. Received: {0}', requestConfig); } - if (!isString(requestConfig.url)) { - throw minErr('$http')('badreq', 'Http request configuration url must be a string. Received: {0}', requestConfig.url); + if (!isString($sce.valueOf(requestConfig.url))) { + throw minErr('$http')('badreq', 'Http request configuration url must be a string or a $sce trusted object. Received: {0}', requestConfig.url); } var config = extend({ @@ -1111,7 +1119,8 @@ function $HttpProvider() { * @description * Shortcut method to perform `GET` request. * - * @param {string} url Relative or absolute URL specifying the destination of the request + * @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 * @returns {HttpPromise} Future object */ @@ -1123,7 +1132,8 @@ function $HttpProvider() { * @description * Shortcut method to perform `DELETE` request. * - * @param {string} url Relative or absolute URL specifying the destination of the request + * @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 * @returns {HttpPromise} Future object */ @@ -1135,7 +1145,8 @@ function $HttpProvider() { * @description * Shortcut method to perform `HEAD` request. * - * @param {string} url Relative or absolute URL specifying the destination of the request + * @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 * @returns {HttpPromise} Future object */ @@ -1146,11 +1157,18 @@ function $HttpProvider() { * * @description * Shortcut method to perform `JSONP` request. - * If you would like to customize where and how the callbacks are stored then try overriding + * + * Note that, since JSONP requests are sensitive because the response is given full acces to the browser, + * the url must be declared, via {@link $sce} as a trusted resource URL. + * You can trust a URL by adding it to the whitelist via + * {@link $sceDelegateProvider#resourceUrlWhitelist `$sceDelegateProvider.resourceUrlWhitelist`} or + * by explicitly trusted the URL via {@link $sce#trustAsResourceUrl `$sce.trustAsResourceUrl(url)`}. + * + * If you would like to customise where and how the callbacks are stored then try overriding * or decorating the {@link $jsonpCallbacks} service. * - * @param {string} url Relative or absolute URL specifying the destination of the request. - * The name of the callback should be the string `JSON_CALLBACK`. + * @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 * @returns {HttpPromise} Future object */ @@ -1249,12 +1267,22 @@ function $HttpProvider() { cache, cachedResp, reqHeaders = config.headers, - url = buildUrl(config.url, config.paramSerializer(config.params)); + url = config.url; + + if (lowercase(config.method) === 'jsonp') { + // JSONP is a pretty sensitive operation where we're allowing a script to have full access to + // our DOM and JS space. So we require that the URL satisfies SCE.RESOURCE_URL. + url = $sce.getTrustedResourceUrl(url); + } else if (!isString(url)) { + // If it is not a string then the URL must be a $sce trusted object + url = $sce.valueOf(url); + } + + url = buildUrl(url, config.paramSerializer(config.params)); $http.pendingRequests.push(config); promise.then(removePendingReq, removePendingReq); - if ((config.cache || defaults.cache) && config.cache !== false && (config.method === 'GET' || config.method === 'JSONP')) { cache = isObject(config.cache) ? config.cache diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index 1e5508deb339..7ba76a898604 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -289,27 +289,48 @@ describe('$http', function() { describe('the instance', function() { - var $httpBackend, $http, $rootScope; + var $httpBackend, $http, $rootScope, $sce; - beforeEach(inject(['$httpBackend', '$http', '$rootScope', function($hb, $h, $rs) { + beforeEach(module(function($sceDelegateProvider) { + // Setup a special whitelisted url that we can use in testing JSONP requests + $sceDelegateProvider.resourceUrlWhitelist(['http://special.whitelisted.resource.com/**']); + })); + + beforeEach(inject(['$httpBackend', '$http', '$rootScope', '$sce', function($hb, $h, $rs, $sc) { $httpBackend = $hb; $http = $h; $rootScope = $rs; + $sce = $sc; spyOn($rootScope, '$apply').and.callThrough(); }])); it('should throw error if the request configuration is not an object', function() { expect(function() { - $http('/url'); + $http('/url'); }).toThrowMinErr('$http','badreq', 'Http request configuration must be an object. Received: /url'); }); - it('should throw error if the request configuration url is not a string', function() { + it('should throw error if the request configuration url is not a string nor a trusted object', function() { + expect(function() { + $http({url: false}); + }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string or a $sce trusted object. Received: false'); + expect(function() { + $http({url: null}); + }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string or a $sce trusted object. Received: null'); + expect(function() { + $http({url: 42}); + }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string or a $sce trusted object. Received: 42'); expect(function() { - $http({url: false}); - }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string. Received: false'); + $http({}); + }).toThrowMinErr('$http','badreq', 'Http request configuration url must be a string or a $sce trusted object. Received: undefined'); }); + it('should accept a $sce trusted object for the request configuration url', function() { + expect(function() { + $httpBackend.expect('GET', '/url').respond(''); + $http({url: $sce.trustAsResourceUrl('/url')}); + }).not.toThrowMinErr('$http','badreq', 'Http request configuration url must be a string. Received: false'); + }); it('should send GET requests if no method specified', function() { $httpBackend.expect('GET', '/url').respond(''); @@ -602,7 +623,7 @@ describe('$http', function() { }); $httpBackend.expect('JSONP', '/some').respond(200); - $http({url: '/some', method: 'JSONP'}).then(callback); + $http({url: $sce.trustAsResourceUrl('/some'), method: 'JSONP'}).then(callback); $httpBackend.flush(); expect(callback).toHaveBeenCalledOnce(); }); @@ -1010,16 +1031,39 @@ describe('$http', function() { it('should have jsonp()', function() { $httpBackend.expect('JSONP', '/url').respond(''); - $http.jsonp('/url'); + $http.jsonp($sce.trustAsResourceUrl('/url')); }); it('jsonp() should allow config param', function() { $httpBackend.expect('JSONP', '/url', undefined, checkHeader('Custom', 'Header')).respond(''); - $http.jsonp('/url', {headers: {'Custom': 'Header'}}); + $http.jsonp($sce.trustAsResourceUrl('/url'), {headers: {'Custom': 'Header'}}); }); }); + describe('jsonp trust', function() { + it('should throw error if the url is not a trusted resource', function() { + var success, error; + $http({method: 'JSONP', url: 'http://example.org/path?cb=JSON_CALLBACK'}).catch( + function(e) { error = e; } + ); + $rootScope.$digest(); + expect(error.message).toContain('[$sce:insecurl]'); + }); + + it('should accept an explicitly trusted resource url', function() { + $httpBackend.expect('JSONP', 'http://example.org/path?cb=JSON_CALLBACK').respond(''); + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?cb=JSON_CALLBACK')}); + }); + + it('jsonp() should accept explictly trusted urls', function() { + $httpBackend.expect('JSONP', '/url').respond(''); + $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url')}); + + $httpBackend.expect('JSONP', '/url?a=b').respond(''); + $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), params: {a: 'b'}}); + }); + }); describe('callbacks', function() { @@ -1481,10 +1525,10 @@ describe('$http', function() { it('should cache JSONP request when cache is provided', inject(function($rootScope) { $httpBackend.expect('JSONP', '/url?cb=JSON_CALLBACK').respond('content'); - $http({method: 'JSONP', url: '/url?cb=JSON_CALLBACK', cache: cache}); + $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url?cb=JSON_CALLBACK'), cache: cache}); $httpBackend.flush(); - $http({method: 'JSONP', url: '/url?cb=JSON_CALLBACK', cache: cache}).success(callback); + $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url?cb=JSON_CALLBACK'), cache: cache}).success(callback); $rootScope.$digest(); expect(callback).toHaveBeenCalledOnce(); From 3c1cd3b1a77722084f344dae8776bced256419c3 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 4 Oct 2016 14:18:41 +0100 Subject: [PATCH 3/3] feat($http): JSONP callback must be specified by `jsonpCallbackParam` config The query parameter that will be used to transmit the JSONP callback to the server is now specified via the `jsonpCallbackParam` config value, instead of using the `JSON_CALLBACK` placeholder. * Any use of `JSON_CALLBACK` in a JSONP request URL will cause an error. * Any request that provides a parameter with the same name as that given by the `jsonpCallbackParam` config property will cause an error. This is to prevent malicious attack via the response from an app inadvertently allowing untrusted data to be used to generate the callback parameter. BREAKING CHANGE You can no longer use the `JSON_CALLBACK` placeholder in your JSONP requests. Instead you must provide the name of the query parameter that will pass the callback via the `jsonpCallbackParam` property of the config object, or app-wide via the `$http.defaults.jsonpCallbackParam` property, which is `"callback"` by default. Before this change: ``` $http.json('trusted/url?callback=JSON_CALLBACK'); $http.json('other/trusted/url', {params:cb:'JSON_CALLBACK'}); ``` After this change: ``` $http.json('trusted/url'); $http.json('other/trusted/url', {callbackParam:'cb'}); ``` --- docs/content/error/$http/badjsonp.ngdoc | 20 +++++++ docs/content/guide/concepts.ngdoc | 2 +- src/ng/http.js | 61 ++++++++++++++++++--- test/ng/httpSpec.js | 73 +++++++++++++++++++------ 4 files changed, 131 insertions(+), 25 deletions(-) create mode 100644 docs/content/error/$http/badjsonp.ngdoc diff --git a/docs/content/error/$http/badjsonp.ngdoc b/docs/content/error/$http/badjsonp.ngdoc new file mode 100644 index 000000000000..18201550bb52 --- /dev/null +++ b/docs/content/error/$http/badjsonp.ngdoc @@ -0,0 +1,20 @@ +@ngdoc error +@name $http:badjsonp +@fullName Bad JSONP Request Configuration +@description + +This error occurs when the URL generated from the configuration object contains a parameter with the +same name as the configured `jsonpCallbackParam` property; or when it contains a parameter whose +value is `JSON_CALLBACK`. + +`$http` JSONP requests need to attach a callback query parameter to the URL. The name of this +parameter is specified in the configuration object (or in the defaults) via the `jsonpCallbackParam` +property. You must not provide your own parameter with this name in the configuratio of the request. + +In previous versions of Angular, you specified where to add the callback parameter value via the +`JSON_CALLBACK` placeholder. This is no longer allowed. + +To resolve this error, remove any parameters that have the same name as the `jsonpCallbackParam`; +and/or remove any parameters that have a value of `JSON_CALLBACK`. + +For more information, see the {@link ng.$http#jsonp `$http.jsonp()`} method API documentation. diff --git a/docs/content/guide/concepts.ngdoc b/docs/content/guide/concepts.ngdoc index 79816c8c12e8..73993d40f0ce 100644 --- a/docs/content/guide/concepts.ngdoc +++ b/docs/content/guide/concepts.ngdoc @@ -326,7 +326,7 @@ The following example shows how this is done with Angular: var YAHOO_FINANCE_URL_PATTERN = '//query.yahooapis.com/v1/public/yql?q=select * from ' + 'yahoo.finance.xchange where pair in ("PAIRS")&format=json&' + - 'env=store://datatables.org/alltableswithkeys&callback=JSON_CALLBACK'; + 'env=store://datatables.org/alltableswithkeys'; var currencies = ['USD', 'EUR', 'CNY']; var usdToForeignRates = {}; diff --git a/src/ng/http.js b/src/ng/http.js index a7b99d809028..27908d5de6e9 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -286,6 +286,10 @@ function $HttpProvider() { * If specified as string, it is interpreted as a function registered with the {@link auto.$injector $injector}. * Defaults to {@link ng.$httpParamSerializer $httpParamSerializer}. * + * - **`defaults.jsonpCallbackParam`** - `{string} - the name of the query parameter that passes the name of the + * callback in a JSONP request. The value of this parameter will be replaced with the expression generated by the + * {@link $jsonpCallbacks} service. Defaults to `'callback'`. + * **/ var defaults = this.defaults = { // transform incoming response data @@ -309,7 +313,9 @@ function $HttpProvider() { xsrfCookieName: 'XSRF-TOKEN', xsrfHeaderName: 'X-XSRF-TOKEN', - paramSerializer: '$httpParamSerializer' + paramSerializer: '$httpParamSerializer', + + jsonpCallbackParam: 'callback' }; var useApplyAsync = false; @@ -869,11 +875,11 @@ function $HttpProvider() { 
http status code: {{status}}
@@ -964,7 +970,8 @@ function $HttpProvider() { method: 'get', transformRequest: defaults.transformRequest, transformResponse: defaults.transformResponse, - paramSerializer: defaults.paramSerializer + paramSerializer: defaults.paramSerializer, + jsonpCallbackParam: defaults.jsonpCallbackParam }, requestConfig); config.headers = mergeHeaders(requestConfig); @@ -1158,11 +1165,27 @@ function $HttpProvider() { * @description * Shortcut method to perform `JSONP` request. * - * Note that, since JSONP requests are sensitive because the response is given full acces to the browser, + * Note that, since JSONP requests are sensitive because the response is given full access to the browser, * the url must be declared, via {@link $sce} as a trusted resource URL. * You can trust a URL by adding it to the whitelist via * {@link $sceDelegateProvider#resourceUrlWhitelist `$sceDelegateProvider.resourceUrlWhitelist`} or - * by explicitly trusted the URL via {@link $sce#trustAsResourceUrl `$sce.trustAsResourceUrl(url)`}. + * by explicitly trusting the URL via {@link $sce#trustAsResourceUrl `$sce.trustAsResourceUrl(url)`}. + * + * 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. + * + * ``` + * $http.jsonp('some/trusted/url', {jsonpCallbackParam: 'callback'}) + * ``` + * + * You can also specify a default callback parameter name in `$http.defaults.jsonpCallbackParam`. + * Initially this is set to `'callback'`. + * + *
+ * You can no longer use the `JSON_CALLBACK` string as a placeholder for specifying where the callback + * parameter value should go. + *
* * If you would like to customise where and how the callbacks are stored then try overriding * or decorating the {@link $jsonpCallbacks} service. @@ -1267,9 +1290,10 @@ function $HttpProvider() { cache, cachedResp, reqHeaders = config.headers, + isJsonp = lowercase(config.method) === 'jsonp', url = config.url; - if (lowercase(config.method) === 'jsonp') { + if (isJsonp) { // JSONP is a pretty sensitive operation where we're allowing a script to have full access to // our DOM and JS space. So we require that the URL satisfies SCE.RESOURCE_URL. url = $sce.getTrustedResourceUrl(url); @@ -1280,6 +1304,11 @@ function $HttpProvider() { url = buildUrl(url, config.paramSerializer(config.params)); + if (isJsonp) { + // Check the url and add the JSONP callback placeholder + url = sanitizeJsonpCallbackParam(url, config.jsonpCallbackParam); + } + $http.pendingRequests.push(config); promise.then(removePendingReq, removePendingReq); @@ -1414,5 +1443,23 @@ 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); + } + + // Add in the JSON_CALLBACK callback param value + url += ((url.indexOf('?') === -1) ? '?' : '&') + key + '=JSON_CALLBACK'; + + return url; + } }]; } diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index 7ba76a898604..da2f26401660 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -326,10 +326,8 @@ describe('$http', function() { }); it('should accept a $sce trusted object for the request configuration url', function() { - expect(function() { - $httpBackend.expect('GET', '/url').respond(''); - $http({url: $sce.trustAsResourceUrl('/url')}); - }).not.toThrowMinErr('$http','badreq', 'Http request configuration url must be a string. Received: false'); + $httpBackend.expect('GET', '/url').respond(''); + $http({url: $sce.trustAsResourceUrl('/url')}); }); it('should send GET requests if no method specified', function() { @@ -622,7 +620,7 @@ describe('$http', function() { expect(r.headers()).toEqual(Object.create(null)); }); - $httpBackend.expect('JSONP', '/some').respond(200); + $httpBackend.expect('JSONP', '/some?callback=JSON_CALLBACK').respond(200); $http({url: $sce.trustAsResourceUrl('/some'), method: 'JSONP'}).then(callback); $httpBackend.flush(); expect(callback).toHaveBeenCalledOnce(); @@ -1030,13 +1028,13 @@ describe('$http', function() { }); it('should have jsonp()', function() { - $httpBackend.expect('JSONP', '/url').respond(''); + $httpBackend.expect('JSONP', '/url?callback=JSON_CALLBACK').respond(''); $http.jsonp($sce.trustAsResourceUrl('/url')); }); it('jsonp() should allow config param', function() { - $httpBackend.expect('JSONP', '/url', undefined, checkHeader('Custom', 'Header')).respond(''); + $httpBackend.expect('JSONP', '/url?callback=JSON_CALLBACK', undefined, checkHeader('Custom', 'Header')).respond(''); $http.jsonp($sce.trustAsResourceUrl('/url'), {headers: {'Custom': 'Header'}}); }); }); @@ -1044,25 +1042,66 @@ describe('$http', function() { describe('jsonp trust', function() { it('should throw error if the url is not a trusted resource', function() { var success, error; - $http({method: 'JSONP', url: 'http://example.org/path?cb=JSON_CALLBACK'}).catch( - function(e) { error = e; } - ); + $http({method: 'JSONP', url: 'http://example.org/path'}) + .catch(function(e) { error = e; }); $rootScope.$digest(); expect(error.message).toContain('[$sce:insecurl]'); }); it('should accept an explicitly trusted resource url', function() { - $httpBackend.expect('JSONP', 'http://example.org/path?cb=JSON_CALLBACK').respond(''); - $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?cb=JSON_CALLBACK')}); + $httpBackend.expect('JSONP', 'http://example.org/path?callback=JSON_CALLBACK').respond(''); + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path')}); }); it('jsonp() should accept explictly trusted urls', function() { - $httpBackend.expect('JSONP', '/url').respond(''); + $httpBackend.expect('JSONP', '/url?callback=JSON_CALLBACK').respond(''); $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url')}); - $httpBackend.expect('JSONP', '/url?a=b').respond(''); + $httpBackend.expect('JSONP', '/url?a=b&callback=JSON_CALLBACK').respond(''); $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), params: {a: 'b'}}); }); + + 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')}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error.message).toContain('[$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.message).toContain('[$http:badjsonp]'); + }); + + it('should error if a param contains a JSON_CALLBACK value', function() { + var error; + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {callback: 'JSON_CALLBACK'}}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error.message).toContain('[$http:badjsonp]'); + + error = undefined; + $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {other: 'JSON_CALLBACK'}}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error.message).toContain('[$http:badjsonp]'); + }); + + 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'}}) + .catch(function(e) { error = e; }); + $rootScope.$digest(); + expect(error.message).toContain('[$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.message).toContain('[$http:badjsonp]'); + }); }); describe('callbacks', function() { @@ -1524,11 +1563,11 @@ describe('$http', function() { })); it('should cache JSONP request when cache is provided', inject(function($rootScope) { - $httpBackend.expect('JSONP', '/url?cb=JSON_CALLBACK').respond('content'); - $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url?cb=JSON_CALLBACK'), cache: cache}); + $httpBackend.expect('JSONP', '/url?callback=JSON_CALLBACK').respond('content'); + $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), cache: cache}); $httpBackend.flush(); - $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url?cb=JSON_CALLBACK'), cache: cache}).success(callback); + $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), cache: cache}).success(callback); $rootScope.$digest(); expect(callback).toHaveBeenCalledOnce();