From bb9ca5a298950d83cbc5f0ea6c1e74cf07edcadb Mon Sep 17 00:00:00 2001 From: Raphael Jamet Date: Tue, 30 Jun 2015 10:36:26 +0200 Subject: [PATCH 1/3] refactor($templateRequest): move $sce checks and trust the cache Move all the calls to $sce.getTrustedUrl inside $templateRequest, and also trust the contents of the cache. This allows prefetching templates and to bypass the checks on things where they make no sense, like templates specified in script tags. Fixes both #12219 and #12220. --- src/ng/compile.js | 2 +- src/ng/directive/ngInclude.js | 6 +++--- src/ng/templateRequest.js | 13 +++++++++++-- src/ngRoute/route.js | 3 +-- test/ng/compileSpec.js | 14 ++++++++++++-- 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 9123d8f7815c..a844f23e10bc 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -2205,7 +2205,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { $compileNode.empty(); - $templateRequest($sce.getTrustedResourceUrl(templateUrl)) + $templateRequest(templateUrl) .then(function(content) { var compileNode, tempTemplateAttrs, $template, childBoundTranscludeFn; diff --git a/src/ng/directive/ngInclude.js b/src/ng/directive/ngInclude.js index c4540c7c1074..528958a943e2 100644 --- a/src/ng/directive/ngInclude.js +++ b/src/ng/directive/ngInclude.js @@ -178,8 +178,8 @@ * @param {Object} angularEvent Synthetic event object. * @param {String} src URL of content to load. */ -var ngIncludeDirective = ['$templateRequest', '$anchorScroll', '$animate', '$sce', - function($templateRequest, $anchorScroll, $animate, $sce) { +var ngIncludeDirective = ['$templateRequest', '$anchorScroll', '$animate', + function($templateRequest, $anchorScroll, $animate) { return { restrict: 'ECA', priority: 400, @@ -215,7 +215,7 @@ var ngIncludeDirective = ['$templateRequest', '$anchorScroll', '$animate', '$sce } }; - scope.$watch($sce.parseAsResourceUrl(srcExp), function ngIncludeWatchAction(src) { + scope.$watch(srcExp, function ngIncludeWatchAction(src) { var afterAnimation = function() { if (isDefined(autoScrollExp) && (!autoScrollExp || scope.$eval(autoScrollExp))) { $anchorScroll(); diff --git a/src/ng/templateRequest.js b/src/ng/templateRequest.js index 5760dde0b973..17ab0d7ae365 100644 --- a/src/ng/templateRequest.js +++ b/src/ng/templateRequest.js @@ -12,7 +12,7 @@ var $compileMinErr = minErr('$compile'); * of the HTTP request is empty, a `$compile` error will be thrown (the exception can be thwarted * by setting the 2nd parameter of the function to true). * - * @param {string} tpl The HTTP request template URL + * @param {string|TrustedResourceUrl} tpl The HTTP request template URL * @param {boolean=} ignoreRequestError Whether or not to ignore the exception when the request fails or the template is empty * * @return {Promise} a promise for the HTTP response data of the given URL. @@ -20,10 +20,19 @@ var $compileMinErr = minErr('$compile'); * @property {number} totalPendingRequests total amount of pending template requests being downloaded. */ function $TemplateRequestProvider() { - this.$get = ['$templateCache', '$http', '$q', function($templateCache, $http, $q) { + this.$get = ['$templateCache', '$http', '$q', '$sce', function($templateCache, $http, $q, $sce) { function handleRequestFn(tpl, ignoreRequestError) { handleRequestFn.totalPendingRequests++; + // We consider the template cache holds only trusted templates, so + // there's no need to go through whitelisting again for keys that already + // are included in there. This also makes Angular accept any script + // directive, no matter its name. However, we still need to unwrap trusted + // types. + if (!isString(tpl) || !$templateCache.get(tpl)) { + tpl = $sce.getTrustedResourceUrl(tpl); + } + var transformResponse = $http.defaults && $http.defaults.transformResponse; if (isArray(transformResponse)) { diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index 1e46fbb33713..28886ff9b8c0 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -591,9 +591,8 @@ function $RouteProvider() { if (angular.isFunction(templateUrl)) { templateUrl = templateUrl(nextRoute.params); } - templateUrl = $sce.getTrustedResourceUrl(templateUrl); if (angular.isDefined(templateUrl)) { - nextRoute.loadedTemplateUrl = templateUrl; + nextRoute.loadedTemplateUrl = $sce.valueOf(templateUrl); template = $templateRequest(templateUrl); } } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index f8e42da02110..2eb3a04b5bb8 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -1296,14 +1296,24 @@ describe('$compile', function() { )); it('should not load cross domain templates by default', inject( - function($compile, $rootScope, $templateCache, $sce) { + function($compile, $httpBackend, $rootScope, $sce) { expect(function() { - $templateCache.put('http://example.com/should-not-load.html', 'Should not load even if in cache.'); $compile('
')($rootScope); }).toThrowMinErr('$sce', 'insecurl', 'Blocked loading resource from url not allowed by $sceDelegate policy. URL: http://example.com/should-not-load.html'); } )); + it('should trust what is already in the template cache', inject( + function($compile, $httpBackend, $rootScope, $templateCache, $sce) { + $httpBackend.expect('GET', 'http://example.com/should-not-load.html').respond('example.com/remote-version'); + $templateCache.put('http://example.com/should-not-load.html', 'example.com/cached-version'); + element = $compile('
')($rootScope); + expect(sortedHtml(element)).toEqual('
'); + $rootScope.$digest(); + expect(sortedHtml(element)).toEqual('
example.com/cached-version
'); + } + )); + it('should load cross domain templates when trusted', inject( function($compile, $httpBackend, $rootScope, $sce) { $httpBackend.expect('GET', 'http://example.com/trusted-template.html').respond('example.com/trusted_template_contents'); From 98bd3ee7aa1aeab2ef12b6609eb4d42db1bd28d3 Mon Sep 17 00:00:00 2001 From: Raphael Jamet Date: Tue, 30 Jun 2015 15:43:32 +0200 Subject: [PATCH 2/3] docs($templateRequest): update the description with caching changes The previous changes to $templateRequest were not documented, they now are. --- src/ng/templateRequest.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ng/templateRequest.js b/src/ng/templateRequest.js index 17ab0d7ae365..6219308ed671 100644 --- a/src/ng/templateRequest.js +++ b/src/ng/templateRequest.js @@ -7,10 +7,12 @@ var $compileMinErr = minErr('$compile'); * @name $templateRequest * * @description - * The `$templateRequest` service downloads the provided template using `$http` and, upon success, - * stores the contents inside of `$templateCache`. If the HTTP request fails or the response data - * of the HTTP request is empty, a `$compile` error will be thrown (the exception can be thwarted - * by setting the 2nd parameter of the function to true). + * The `$templateRequest` service runs security checks then downloads the provided template using + * `$http` and, upon success, stores the contents inside of `$templateCache`. If the HTTP request + * fails or the response data of the HTTP request is empty, a `$compile` error will be thrown (the + * exception can be thwarted by setting the 2nd parameter of the function to true). Note that the + * contents of `$templateCache` are trusted, so the call to `$sce.getTrustedUrl(tpl)` is omitted + * when `tpl` is of type string and `$templateCache` has the matching entry. * * @param {string|TrustedResourceUrl} tpl The HTTP request template URL * @param {boolean=} ignoreRequestError Whether or not to ignore the exception when the request fails or the template is empty From 51e81dd3c79d7511de28c18b7a66c66aeef9803c Mon Sep 17 00:00:00 2001 From: Raphael Jamet Date: Tue, 30 Jun 2015 20:09:34 +0200 Subject: [PATCH 3/3] fix($templateRequest): Remove useless dependencies in tests --- test/ng/compileSpec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 2eb3a04b5bb8..c6e49603b4f4 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -1296,7 +1296,7 @@ describe('$compile', function() { )); it('should not load cross domain templates by default', inject( - function($compile, $httpBackend, $rootScope, $sce) { + function($compile, $rootScope) { expect(function() { $compile('
')($rootScope); }).toThrowMinErr('$sce', 'insecurl', 'Blocked loading resource from url not allowed by $sceDelegate policy. URL: http://example.com/should-not-load.html'); @@ -1304,7 +1304,7 @@ describe('$compile', function() { )); it('should trust what is already in the template cache', inject( - function($compile, $httpBackend, $rootScope, $templateCache, $sce) { + function($compile, $httpBackend, $rootScope, $templateCache) { $httpBackend.expect('GET', 'http://example.com/should-not-load.html').respond('example.com/remote-version'); $templateCache.put('http://example.com/should-not-load.html', 'example.com/cached-version'); element = $compile('
')($rootScope);