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

fix(ngRoute): wrong redirect to paths containing optional or special parameters #5746

Closed
wants to merge 2 commits into from

Conversation

Pasvaz
Copy link

@Pasvaz Pasvaz commented Jan 11, 2014

When the path contains optional parameters ? or special parameters * the redirects to those routes are broken, the location will end with the relative encoded character.

To reproduce the issue:

  • create a route with optional parameters, es. $routeProvider.when('/profile/:userId?', ...)
  • angular will add the automatic redirect with or without the trailing /, in this case '/profile/:userId?/'
  • point your browser to the redirect http://localhost/#!/profile/0/, you'll see the encoded ? (%3F) at the end of the url: http://localhost/#!/profile/0%3F

Happens also for the special parameters * and also for the opposite scenario, when the route has the ending / (/profile/:userId?/) and you browse the url without slash /profile/0. It affects every redirect that uses the function interpolate in combination with special parameters.

…ith star or question mark

When the path contains optional parameters or special parameters the redirects to those routes are broken, the location will end with the relative encoded character.
To reproduce the issue:
* create a route with optional parameters, es. `$routeProvider.when('/profile/:userId?', ...)`
* angular will add the automatic redirect with or without the trailing **/**, in this case `'/profile/:userId?/'`
* point the browser to the redirect `http://localhost/#!/profile/0/`, you'll see the encoded `? (%3F)` at the end of the url: `http://localhost/#!/profile/0%3F`

Happens also for the special parameters `*` and also for the opposite scenario, when the route has `/profile/:userId?/` and you browse the url `/profile/0`. It affects every redirect that uses the function interpolate.
@@ -586,10 +586,10 @@ function $RouteProvider(){
if (i === 0) {
result.push(segment);
} else {
var segmentMatch = segment.match(/(\w+)(.*)/);
var segmentMatch = segment.match(/(\w+)([\?|\*])?(.*)/);
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need the pipe in the square brackets

Copy link
Author

Choose a reason for hiding this comment

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

I thought it was a third special operator because it was used in the function pathRegExp, the part of code you have just updated with the #5920
I'll remove it as soon as I'll be at my pc.

@caitp
Copy link
Contributor

caitp commented Jan 21, 2014

Could you show a test case which fails without this change? I'm just trying to wrap my head around what the issue is --- NVM just read your explanation, makes sense. Thanks!

@IgorMinar
Copy link
Contributor

we need a unit test for this change.

avdv added a commit to avdv/angular.js that referenced this pull request Jul 15, 2014
@avdv
Copy link

avdv commented Jul 15, 2014

I stumbled over the same problem.

There's a small problem with your fix here too and so I rewrote the interpolation function and added a test case: #8200

avdv added a commit to avdv/angular.js that referenced this pull request Jul 15, 2014
When the redirect path contains optional or special parameters the
interpolation function misses to remove them.

This is also the case for the auto-generated redirection where a slash
is appended or removed to a route.

Change the way parameters are interpolated in a string. Remove undefined
optional named groups and properly prepend a slash in case the named
group was starting with one.

Related to angular#5746.
avdv added a commit to avdv/angular.js that referenced this pull request Jul 15, 2014
When the redirect path contains optional or special parameters the
interpolation function misses to remove them.

This is also the case for the auto-generated redirection where a slash
is appended or removed to a route.

Change the way parameters are interpolated in a string. Remove undefined
optional named groups and properly prepend a slash in case the named
group was starting with one.

Related to angular#5746.
avdv added a commit to avdv/angular.js that referenced this pull request Aug 4, 2014
When the redirect path contains optional or special parameters the
interpolation function misses to remove them.

This is also the case for the auto-generated redirection where a slash
is appended or removed to a route.

Change the way parameters are interpolated in a string. Remove undefined
optional named groups and properly prepend a slash in case the named
group was starting with one.

Related to angular#5746.
avdv added a commit to avdv/angular.js that referenced this pull request Aug 20, 2014
When the redirect path contains optional or special parameters the
interpolation function misses to remove them.

This is also the case for the auto-generated redirection where a slash
is appended or removed to a route.

Change the way parameters are interpolated in a string. Remove undefined
optional named groups and properly prepend a slash in case the named
group was starting with one.

Related to angular#5746.
@btford btford removed the gh: PR label Aug 20, 2014
@jeffbcross jeffbcross force-pushed the master branch 2 times, most recently from cad9560 to f294244 Compare October 2, 2014 22:08
@jeffbcross jeffbcross force-pushed the master branch 4 times, most recently from e8dc429 to e83fab9 Compare October 10, 2014 17:37
@gkalpak
Copy link
Member

gkalpak commented Nov 23, 2014

This has been fixed by #9827.

@pkozlowski-opensource
Copy link
Member

Indeed, thnx @gkalpak !

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.

7 participants