Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($routeProvider): do not deep-copy route definition objects #14750

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions angularFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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'
Expand Down
26 changes: 0 additions & 26 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
includes: true,
arrayRemove: true,
copy: true,
shallowCopy: true,
equals: true,
csp: true,
jq: true,
Expand Down Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion src/ngRoute/route.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
'use strict';

/* global shallowCopy: false */

// There are necessary for `shallowCopy()` (included via `src/shallowCopy.js`)
var isArray = angular.isArray;
var isObject = angular.isObject;

/**
* @ngdoc module
* @name ngRoute
Expand Down Expand Up @@ -160,7 +166,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 = shallowCopy(route);
if (angular.isUndefined(routeCopy.reloadOnSearch)) {
routeCopy.reloadOnSearch = true;
}
Expand Down
28 changes: 28 additions & 0 deletions src/shallowCopy.js
Original file line number Diff line number Diff line change
@@ -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;
}
2 changes: 1 addition & 1 deletion src/stringify.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

/* global: toDebugString: true */
/* global toDebugString: true */

function serializeObject(obj) {
var seen = [];
Expand Down
81 changes: 81 additions & 0 deletions test/ngRoute/routeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the $routeProvider registration in the inject and not the module block?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I couldn't access $sce during config 😟


$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) {
Expand Down