Skip to content
This repository has been 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

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Jun 9, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix

What is the current behavior? (You can also link to an open issue here)
When registering a route, route definition objects are deep-copied, which breaks specific custom implementations of $sce (used to trust a templateUrl as RESOURCE_URL).

What is the new behavior (if this is a feature change)?
When registering a route, route definition objects are shallow-copied, which doesn't break custom implementations of $sce.

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

Other information:
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 #14699 (comment), 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

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,6 +1,6 @@
'use strict';

describe('$route', function() {
fdescribe('$route', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

fdescribe

- Remove `fdescribe`.

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 😟

@Narretz
Copy link
Contributor

Narretz commented Jun 10, 2016

LGTM. If you needed another one. :>

- Re-use the `shallowCopy` function from core.
@petebacondarwin
Copy link
Contributor

LGTM

- Make jsHint happy.
@gkalpak gkalpak closed this in 53ab8bc Jun 10, 2016
gkalpak added a commit that referenced this pull request Jun 10, 2016
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 #14699 (comment),
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

Closes #14750
@gkalpak gkalpak deleted the fix-ngRoute-shallow-copy-rdo branch June 10, 2016 11:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$routeProvider deep-copies its route objects before using them, which is probably not necessary
5 participants