Skip to content

Commit e7a910f

Browse files
chirayukpetebacondarwin
authored andcommitted
feat(http): JSONP requests now require trusted resource URLs
Reject JSONP requests that are not trusted by `$sce` as "ResourceUrl". Closes angular#11352 BREAKING CHANGE All JSONP requests now require the URL to be whitelisted 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?**` ]); }]); ```
1 parent 32aa7e7 commit e7a910f

File tree

4 files changed

+43
-17
lines changed

4 files changed

+43
-17
lines changed

src/ng/http.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -379,8 +379,8 @@ function $HttpProvider() {
379379
**/
380380
var interceptorFactories = this.interceptors = [];
381381

382-
this.$get = ['$browser', '$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector',
383-
function($browser, $httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) {
382+
this.$get = ['$sce', '$browser', '$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector',
383+
function($sce, $browser, $httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) {
384384

385385
var defaultCache = $cacheFactory('$http');
386386

src/ng/httpBackend.js

+8-3
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,19 @@ function $xhrFactoryProvider() {
4949
* $httpBackend} which can be trained with responses.
5050
*/
5151
function $HttpBackendProvider() {
52-
this.$get = ['$browser', '$jsonpCallbacks', '$document', '$xhrFactory', function($browser, $jsonpCallbacks, $document, $xhrFactory) {
53-
return createHttpBackend($browser, $xhrFactory, $browser.defer, $jsonpCallbacks, $document[0]);
52+
this.$get = ['$sce', '$browser', '$jsonpCallbacks', '$document', '$xhrFactory', function($sce, $browser, $jsonpCallbacks, $document, $xhrFactory) {
53+
return createHttpBackend($sce, $browser, $xhrFactory, $browser.defer, $jsonpCallbacks, $document[0]);
5454
}];
5555
}
5656

57-
function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDocument) {
57+
function createHttpBackend($sce, $browser, createXhr, $browserDefer, callbacks, rawDocument) {
5858
// TODO(vojta): fix the signature
5959
return function(method, url, post, callback, headers, timeout, withCredentials, responseType, eventHandlers, uploadEventHandlers) {
60+
if (lowercase(method) == 'jsonp') {
61+
// This is a pretty sensitive operation where we're allowing a script to have full access to
62+
// our DOM and JS space. So we require that the URL satisfies SCE.RESOURCE_URL.
63+
url = $sce.getTrustedResourceUrl(url);
64+
}
6065
url = url || $browser.url();
6166

6267
if (lowercase(method) === 'jsonp') {

test/ng/httpBackendSpec.js

+30-10
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,16 @@
33

44
describe('$httpBackend', function() {
55

6-
var $backend, $browser, $jsonpCallbacks,
6+
var $sce, $backend, $browser, $jsonpCallbacks,
77
xhr, fakeDocument, callback;
88

9-
beforeEach(inject(function($injector) {
9+
beforeEach(module(function($sceDelegateProvider) {
10+
// Setup a special whitelisted url that we can use in testing JSONP requests
11+
$sceDelegateProvider.resourceUrlWhitelist(['http://special.whitelisted.resource.com/**']);
12+
}));
1013

14+
beforeEach(inject(function($injector) {
15+
$sce = $injector.get('$sce');
1116
$browser = $injector.get('$browser');
1217

1318
fakeDocument = {
@@ -48,7 +53,7 @@ describe('$httpBackend', function() {
4853
}
4954
};
5055

51-
$backend = createHttpBackend($browser, createMockXhr, $browser.defer, $jsonpCallbacks, fakeDocument);
56+
$backend = createHttpBackend($sce, $browser, createMockXhr, $browser.defer, $jsonpCallbacks, fakeDocument);
5257
callback = jasmine.createSpy('done');
5358
}));
5459

@@ -273,7 +278,7 @@ describe('$httpBackend', function() {
273278

274279
it('should call $xhrFactory with method and url', function() {
275280
var mockXhrFactory = jasmine.createSpy('mockXhrFactory').and.callFake(createMockXhr);
276-
$backend = createHttpBackend($browser, mockXhrFactory, $browser.defer, $jsonpCallbacks, fakeDocument);
281+
$backend = createHttpBackend($sce, $browser, mockXhrFactory, $browser.defer, $jsonpCallbacks, fakeDocument);
277282
$backend('GET', '/some-url', 'some-data', noop);
278283
expect(mockXhrFactory).toHaveBeenCalledWith('GET', '/some-url');
279284
});
@@ -334,20 +339,20 @@ describe('$httpBackend', function() {
334339

335340
var SCRIPT_URL = /([^\?]*)\?cb=(.*)/;
336341

337-
338342
it('should add script tag for JSONP request', function() {
339343
callback.and.callFake(function(status, response) {
340344
expect(status).toBe(200);
341345
expect(response).toBe('some-data');
342346
});
343347

344-
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
348+
$backend('JSONP', 'http://special.whitelisted.resource.com/path?cb=JSON_CALLBACK', null, callback);
349+
345350
expect(fakeDocument.$$scripts.length).toBe(1);
346351

347352
var script = fakeDocument.$$scripts.shift(),
348353
url = script.src.match(SCRIPT_URL);
349354

350-
expect(url[1]).toBe('http://example.org/path');
355+
expect(url[1]).toBe('http://special.whitelisted.resource.com/path');
351356
$jsonpCallbacks[url[2]]('some-data');
352357
browserTrigger(script, 'load');
353358

@@ -358,7 +363,8 @@ describe('$httpBackend', function() {
358363
it('should clean up the callback and remove the script', function() {
359364
spyOn($jsonpCallbacks, 'removeCallback').and.callThrough();
360365

361-
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
366+
$backend('JSONP', 'http://special.whitelisted.resource.com/path?cb=JSON_CALLBACK', null, callback);
367+
362368
expect(fakeDocument.$$scripts.length).toBe(1);
363369

364370

@@ -375,6 +381,7 @@ describe('$httpBackend', function() {
375381

376382
it('should set url to current location if not specified or empty string', function() {
377383
$backend('JSONP', undefined, null, callback);
384+
378385
expect(fakeDocument.$$scripts[0].src).toBe($browser.url());
379386
fakeDocument.$$scripts.shift();
380387

@@ -390,7 +397,8 @@ describe('$httpBackend', function() {
390397
expect(status).toBe(-1);
391398
});
392399

393-
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback, null, 2000);
400+
$backend('JSONP', 'http://special.whitelisted.resource.com/path?cb=JSON_CALLBACK', null, callback, null, 2000);
401+
394402
expect(fakeDocument.$$scripts.length).toBe(1);
395403
expect($browser.deferredFns[0].time).toBe(2000);
396404

@@ -405,6 +413,18 @@ describe('$httpBackend', function() {
405413
});
406414

407415

416+
it('should throw error if the url is not a trusted resource', function() {
417+
expect(function() {
418+
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
419+
}).toThrowMinErr('$sce', 'insecurl');
420+
});
421+
422+
it('should not throw error if the url is an explicitly trusted resource', function() {
423+
expect(function() {
424+
$backend('JSONP', $sce.trustAsResourceUrl('http://example.org/path?cb=JSON_CALLBACK'), null, callback);
425+
}).not.toThrowMinErr('$sce', 'insecurl');
426+
});
427+
408428
// TODO(vojta): test whether it fires "async-start"
409429
// TODO(vojta): test whether it fires "async-end" on both success and error
410430
});
@@ -420,7 +440,7 @@ describe('$httpBackend', function() {
420440
}
421441

422442
beforeEach(function() {
423-
$backend = createHttpBackend($browser, createMockXhr);
443+
$backend = createHttpBackend($sce, $browser, createMockXhr);
424444
});
425445

426446

test/ngRoute/routeSpec.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -1016,9 +1016,10 @@ describe('$route', function() {
10161016
$routeProvider = _$routeProvider_;
10171017

10181018
$provide.decorator('$sce', function($delegate) {
1019+
function getVal(v) { return v.getVal ? v.getVal() : v; }
10191020
$delegate.trustAsResourceUrl = function(url) { return new MySafeResourceUrl(url); };
1020-
$delegate.getTrustedResourceUrl = function(v) { return v.getVal(); };
1021-
$delegate.valueOf = function(v) { return v.getVal(); };
1021+
$delegate.getTrustedResourceUrl = function(v) { return getVal(v); };
1022+
$delegate.valueOf = function(v) { return getVal(v); };
10221023
return $delegate;
10231024
});
10241025
});

0 commit comments

Comments
 (0)