From f4b882c8c30d0d0d68fe04ed65bfca1640cfe0cf Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Thu, 9 Jun 2016 23:56:25 +0300 Subject: [PATCH 1/4] fix($routeProvider): do not deep-copy route definition objects Deep-copying route definition objects can break specific custom implementations of `$sce` (used to trust a `templateUrl` as RESOURCE_URL). The purpose of copying route definition objects was to guard against the user's modifying the route definition object after route registration, while still capturing inherited properties. As suggested by @IgorMinar in https://github.com/angular/angular.js/pull/14699#discussion_r66480539, we can achieve both _and_ support custom `$sce` implementations, by shallow-copying instead. This is an alternative implementation for #14699, which avoids the breaking change. Fixes 14478 Closes 14699 --- src/ngRoute/route.js | 13 +++++- test/ngRoute/routeSpec.js | 83 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index cf5256c3dd51..d3ea7439cb4f 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -39,6 +39,17 @@ function $RouteProvider() { return angular.extend(Object.create(parent), extra); } + function createShallowCopy(src) { + if (!angular.isObject(src)) return src; + + var dst = {}; + for (var key in src) { + dst[key] = src[key]; + } + + return dst; + } + var routes = {}; /** @@ -160,7 +171,7 @@ function $RouteProvider() { */ this.when = function(path, route) { //copy original route object to preserve params inherited from proto chain - var routeCopy = angular.copy(route); + var routeCopy = createShallowCopy(route); if (angular.isUndefined(routeCopy.reloadOnSearch)) { routeCopy.reloadOnSearch = true; } diff --git a/test/ngRoute/routeSpec.js b/test/ngRoute/routeSpec.js index aa1d283fda83..881fe870af0d 100644 --- a/test/ngRoute/routeSpec.js +++ b/test/ngRoute/routeSpec.js @@ -1,6 +1,6 @@ 'use strict'; -describe('$route', function() { +fdescribe('$route', function() { var $httpBackend, element; @@ -900,6 +900,87 @@ describe('$route', function() { }); + it('should not get affected by modifying the route definition object after route registration', + function() { + module(function($routeProvider) { + var rdo = {}; + + rdo.templateUrl = 'foo.html'; + $routeProvider.when('/foo', rdo); + + rdo.templateUrl = 'bar.html'; + $routeProvider.when('/bar', rdo); + }); + + inject(function($location, $rootScope, $route) { + $location.path('/bar'); + $rootScope.$digest(); + expect($location.path()).toBe('/bar'); + expect($route.current.templateUrl).toBe('bar.html'); + + $location.path('/foo'); + $rootScope.$digest(); + expect($location.path()).toBe('/foo'); + expect($route.current.templateUrl).toBe('foo.html'); + }); + } + ); + + + it('should use the property values of the passed in route definition object directly', + function() { + var $routeProvider; + + module(function(_$routeProvider_) { + $routeProvider = _$routeProvider_; + }); + + inject(function($location, $rootScope, $route, $sce) { + var sceWrappedUrl = $sce.trustAsResourceUrl('foo.html'); + $routeProvider.when('/foo', {templateUrl: sceWrappedUrl}); + + $location.path('/foo'); + $rootScope.$digest(); + expect($location.path()).toBe('/foo'); + expect($route.current.templateUrl).toBe(sceWrappedUrl); + }); + } + ); + + + it('should support custom `$sce` implementations', function() { + function MySafeResourceUrl(val) { + var self = this; + this._val = val; + this.getVal = function() { + return (this !== self) ? null : this._val; + }; + } + + var $routeProvider; + + module(function($provide, _$routeProvider_) { + $routeProvider = _$routeProvider_; + + $provide.decorator('$sce', function($delegate) { + $delegate.trustAsResourceUrl = function(url) { return new MySafeResourceUrl(url); }; + $delegate.getTrustedResourceUrl = function(v) { return v.getVal(); }; + $delegate.valueOf = function(v) { return v.getVal(); }; + return $delegate; + }); + }); + + inject(function($location, $rootScope, $route, $sce) { + $routeProvider.when('/foo', {templateUrl: $sce.trustAsResourceUrl('foo.html')}); + + $location.path('/foo'); + $rootScope.$digest(); + expect($location.path()).toBe('/foo'); + expect($sce.valueOf($route.current.templateUrl)).toBe('foo.html'); + }); + }); + + describe('redirection', function() { it('should support redirection via redirectTo property by updating $location', function() { module(function($routeProvider) { From f18792836a913aa373c612b32e0b4c30a2414091 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 10 Jun 2016 00:25:45 +0300 Subject: [PATCH 2/4] fixup 1 - Remove `fdescribe`. --- test/ngRoute/routeSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ngRoute/routeSpec.js b/test/ngRoute/routeSpec.js index 881fe870af0d..aeb559eb5664 100644 --- a/test/ngRoute/routeSpec.js +++ b/test/ngRoute/routeSpec.js @@ -1,6 +1,6 @@ 'use strict'; -fdescribe('$route', function() { +describe('$route', function() { var $httpBackend, element; From fdcbc50a3497e2fe31e0091a90ecb60436f1a726 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 10 Jun 2016 13:17:50 +0300 Subject: [PATCH 3/4] fixup 2 - Re-use the `shallowCopy` function from core. --- angularFiles.js | 2 ++ src/Angular.js | 26 -------------------------- src/ngRoute/route.js | 17 +++++------------ src/shallowCopy.js | 28 ++++++++++++++++++++++++++++ 4 files changed, 35 insertions(+), 38 deletions(-) create mode 100644 src/shallowCopy.js diff --git a/angularFiles.js b/angularFiles.js index 35f302cee0d5..f1fda79e1cb6 100755 --- a/angularFiles.js +++ b/angularFiles.js @@ -5,6 +5,7 @@ var angularFiles = { 'src/minErr.js', 'src/Angular.js', 'src/loader.js', + 'src/shallowCopy.js', 'src/stringify.js', 'src/AngularPublic.js', 'src/jqLite.js', @@ -128,6 +129,7 @@ var angularFiles = { 'src/ngResource/resource.js' ], 'ngRoute': [ + 'src/shallowCopy.js', 'src/ngRoute/route.js', 'src/ngRoute/routeParams.js', 'src/ngRoute/directive/ngView.js' diff --git a/src/Angular.js b/src/Angular.js index bbb603f82f11..ddbe72f5600d 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -57,7 +57,6 @@ includes: true, arrayRemove: true, copy: true, - shallowCopy: true, equals: true, csp: true, jq: true, @@ -933,31 +932,6 @@ function copy(source, destination) { } } -/** - * Creates a shallow copy of an object, an array or a primitive. - * - * Assumes that there are no proto properties for objects. - */ -function shallowCopy(src, dst) { - if (isArray(src)) { - dst = dst || []; - - for (var i = 0, ii = src.length; i < ii; i++) { - dst[i] = src[i]; - } - } else if (isObject(src)) { - dst = dst || {}; - - for (var key in src) { - if (!(key.charAt(0) === '$' && key.charAt(1) === '$')) { - dst[key] = src[key]; - } - } - } - - return dst || src; -} - /** * @ngdoc function diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index d3ea7439cb4f..08894a5e4d14 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -1,5 +1,9 @@ 'use strict'; +// There are necessary for `shallowCopy()` (included via `src/shallowCopy.js`) +var isArray = angular.isArray; +var isObject = angular.isObject; + /** * @ngdoc module * @name ngRoute @@ -39,17 +43,6 @@ function $RouteProvider() { return angular.extend(Object.create(parent), extra); } - function createShallowCopy(src) { - if (!angular.isObject(src)) return src; - - var dst = {}; - for (var key in src) { - dst[key] = src[key]; - } - - return dst; - } - var routes = {}; /** @@ -171,7 +164,7 @@ function $RouteProvider() { */ this.when = function(path, route) { //copy original route object to preserve params inherited from proto chain - var routeCopy = createShallowCopy(route); + var routeCopy = shallowCopy(route); if (angular.isUndefined(routeCopy.reloadOnSearch)) { routeCopy.reloadOnSearch = true; } diff --git a/src/shallowCopy.js b/src/shallowCopy.js new file mode 100644 index 000000000000..3c58d92f7791 --- /dev/null +++ b/src/shallowCopy.js @@ -0,0 +1,28 @@ +'use strict'; + +/* global: shallowCopy: true */ + +/** + * Creates a shallow copy of an object, an array or a primitive. + * + * Assumes that there are no proto properties for objects. + */ +function shallowCopy(src, dst) { + if (isArray(src)) { + dst = dst || []; + + for (var i = 0, ii = src.length; i < ii; i++) { + dst[i] = src[i]; + } + } else if (isObject(src)) { + dst = dst || {}; + + for (var key in src) { + if (!(key.charAt(0) === '$' && key.charAt(1) === '$')) { + dst[key] = src[key]; + } + } + } + + return dst || src; +} From 3a548f3aece8d0cf5a80190a31485a423798e7e7 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 10 Jun 2016 13:46:25 +0300 Subject: [PATCH 4/4] fixup 3 - Make jsHint happy. --- src/ngRoute/route.js | 2 ++ src/shallowCopy.js | 2 +- src/stringify.js | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index 08894a5e4d14..c06c26475d1f 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -1,5 +1,7 @@ 'use strict'; +/* global shallowCopy: false */ + // There are necessary for `shallowCopy()` (included via `src/shallowCopy.js`) var isArray = angular.isArray; var isObject = angular.isObject; diff --git a/src/shallowCopy.js b/src/shallowCopy.js index 3c58d92f7791..36602e55680e 100644 --- a/src/shallowCopy.js +++ b/src/shallowCopy.js @@ -1,6 +1,6 @@ 'use strict'; -/* global: shallowCopy: true */ +/* global shallowCopy: true */ /** * Creates a shallow copy of an object, an array or a primitive. diff --git a/src/stringify.js b/src/stringify.js index 2a39da697048..ce07dffa4092 100644 --- a/src/stringify.js +++ b/src/stringify.js @@ -1,6 +1,6 @@ 'use strict'; -/* global: toDebugString: true */ +/* global toDebugString: true */ function serializeObject(obj) { var seen = [];