Skip to content

Commit f4b882c

Browse files
committed
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 angular#14699 (comment), we can achieve both _and_ support custom `$sce` implementations, by shallow-copying instead. This is an alternative implementation for angular#14699, which avoids the breaking change. Fixes 14478 Closes 14699
1 parent 3ba29c4 commit f4b882c

File tree

2 files changed

+94
-2
lines changed

2 files changed

+94
-2
lines changed

src/ngRoute/route.js

+12-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,17 @@ function $RouteProvider() {
3939
return angular.extend(Object.create(parent), extra);
4040
}
4141

42+
function createShallowCopy(src) {
43+
if (!angular.isObject(src)) return src;
44+
45+
var dst = {};
46+
for (var key in src) {
47+
dst[key] = src[key];
48+
}
49+
50+
return dst;
51+
}
52+
4253
var routes = {};
4354

4455
/**
@@ -160,7 +171,7 @@ function $RouteProvider() {
160171
*/
161172
this.when = function(path, route) {
162173
//copy original route object to preserve params inherited from proto chain
163-
var routeCopy = angular.copy(route);
174+
var routeCopy = createShallowCopy(route);
164175
if (angular.isUndefined(routeCopy.reloadOnSearch)) {
165176
routeCopy.reloadOnSearch = true;
166177
}

test/ngRoute/routeSpec.js

+82-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
describe('$route', function() {
3+
fdescribe('$route', function() {
44
var $httpBackend,
55
element;
66

@@ -900,6 +900,87 @@ describe('$route', function() {
900900
});
901901

902902

903+
it('should not get affected by modifying the route definition object after route registration',
904+
function() {
905+
module(function($routeProvider) {
906+
var rdo = {};
907+
908+
rdo.templateUrl = 'foo.html';
909+
$routeProvider.when('/foo', rdo);
910+
911+
rdo.templateUrl = 'bar.html';
912+
$routeProvider.when('/bar', rdo);
913+
});
914+
915+
inject(function($location, $rootScope, $route) {
916+
$location.path('/bar');
917+
$rootScope.$digest();
918+
expect($location.path()).toBe('/bar');
919+
expect($route.current.templateUrl).toBe('bar.html');
920+
921+
$location.path('/foo');
922+
$rootScope.$digest();
923+
expect($location.path()).toBe('/foo');
924+
expect($route.current.templateUrl).toBe('foo.html');
925+
});
926+
}
927+
);
928+
929+
930+
it('should use the property values of the passed in route definition object directly',
931+
function() {
932+
var $routeProvider;
933+
934+
module(function(_$routeProvider_) {
935+
$routeProvider = _$routeProvider_;
936+
});
937+
938+
inject(function($location, $rootScope, $route, $sce) {
939+
var sceWrappedUrl = $sce.trustAsResourceUrl('foo.html');
940+
$routeProvider.when('/foo', {templateUrl: sceWrappedUrl});
941+
942+
$location.path('/foo');
943+
$rootScope.$digest();
944+
expect($location.path()).toBe('/foo');
945+
expect($route.current.templateUrl).toBe(sceWrappedUrl);
946+
});
947+
}
948+
);
949+
950+
951+
it('should support custom `$sce` implementations', function() {
952+
function MySafeResourceUrl(val) {
953+
var self = this;
954+
this._val = val;
955+
this.getVal = function() {
956+
return (this !== self) ? null : this._val;
957+
};
958+
}
959+
960+
var $routeProvider;
961+
962+
module(function($provide, _$routeProvider_) {
963+
$routeProvider = _$routeProvider_;
964+
965+
$provide.decorator('$sce', function($delegate) {
966+
$delegate.trustAsResourceUrl = function(url) { return new MySafeResourceUrl(url); };
967+
$delegate.getTrustedResourceUrl = function(v) { return v.getVal(); };
968+
$delegate.valueOf = function(v) { return v.getVal(); };
969+
return $delegate;
970+
});
971+
});
972+
973+
inject(function($location, $rootScope, $route, $sce) {
974+
$routeProvider.when('/foo', {templateUrl: $sce.trustAsResourceUrl('foo.html')});
975+
976+
$location.path('/foo');
977+
$rootScope.$digest();
978+
expect($location.path()).toBe('/foo');
979+
expect($sce.valueOf($route.current.templateUrl)).toBe('foo.html');
980+
});
981+
});
982+
983+
903984
describe('redirection', function() {
904985
it('should support redirection via redirectTo property by updating $location', function() {
905986
module(function($routeProvider) {

0 commit comments

Comments
 (0)