Angular.js: AngularJS unescapes matrix parameters

[bourey]

I'm submitting a ...

• [x] bug report
• [ ] feature request
• [ ] other (Please do not submit support requests here (see above))

Current behavior:
AngularJS un-escapes matrix parameters, preventing matrix parameters with slashes from being used in dual-router AngularJS / Angular applications.

Expected / new behavior:
AngularJS should not un-escape matrix parameters.

Minimal reproduction of the problem with instructions:
In a hybrid setup, add a link like: ANGULAR A
While Angular properly renders this link as "http://localhost:4200/a/ng2;path=%2Fsome%2Fpath", AngularJS rewrites it as http://localhost:4200/a/ng2;path=/some/path during navigation. Depending on the setup, this causes the application to display either a missing route mapping or a redirect loop.

AngularJS version: 1.5+

Browser: all

[petebacondarwin]

This sounds like a valid bug. Thanks for reporting @bourey !

[petebacondarwin]

Is it $location or $route that is doing the rewriting?

[jasonaden]

@petebacondarwin I think it's $location. I've looked at this encoding/decoding before and I think it's all from $location.

[petebacondarwin]

I think the exact problem here is that we are unescaping forward slashes, right?
We have tests that check that we unescape other special characters.
Can anyone point me to the spec for these matrix params? I can't find anything official.

[petebacondarwin]

I have created a PR for this: #16316 - please take a look. As with all things related to the can of worms that is $location fixing this bug could well create problems with loads of other use cases.

[hanoii]

I just want to flag, without much information as I don't have it right now but finally found out that this update was causing of an issue we were having with https://github.com/angular-ui/ui-router which wasn't there before.

For some reason $state.go('catchall', {alias: URL}) we have setup stopped to work and encoded URLs appeared on the URL breaking the route handling altogether. It might be some inconsistency with uirouter or something else entirely specific to my app but as it was triggered by this update (and very likely this issue) wanted to note it.

[petebacondarwin]

@hanoii - perhaps this regression fix will solve your problem: #16316 ?

[hanoii]

@petebacondarwin I just tried master and it didn't fix my problem. Not sure if I am testing it correctly. I copied over the built angular.js and angular-sanitize.js, but not the other bits, but I think the issue is within angular.js anyway.

[dhoko]

Hey :wave: ,
This fix https://github.com/angular/angular.js/pull/16350/files#diff-06a39a7c214933c36a5c4b080ef4d56dR425 doesn't work for me, my app is still broken. (with ui-routeur 1.0.11)

Why not using decodeURIComponent instead of decodeURI for HTML5 mode too ?

[petebacondarwin]

@dhoko - I think I agree. See https://github.com/angular/angular.js/pull/16354

[dhoko]

This fix works, thx :+1:

[Narretz]

@bourey can you check if your use case still works with AngularJS master + this patch https://github.com/angular/angular.js/pull/16354? We had to make some changes so that other use cases do not break.

[hanoii]

@Narretz at least I tried it and the issue I was having on https://github.com/angular/angular.js/issues/16312#issuecomment-348023449 seems to have gone with master + patch #16354

[phrough]

I am definitely seeing issues related to this with AngularJS 1.7.2 and legacy ui-router 0.4.3. In html5 mode, trying to access a url directly gets rerouted to my fallback state because the the slashes are converted to %2F. Rolling back this change results in expected behavior. The deep-linked state loads as expected.

EDIT: In my particular case, the problem observed was related to the change in default hashPrefix. Setting it to its previous value of '' in config resolved the issue.
$locationProvider.hashPrefix('');

[binki]

I personally don’t think that / should be treated as such a special case here.

RFC3986§6.2.2.2 recommends that percent normalization only the normalize unreserved characters listed in RFC3986§2.3. These unreserved characters are -, ., _, ~, ALPHA (/[a-zA-Z]/), and DIGIT (/[0-9]/).

RFC3986§3.3 permits the entirety of the path to consist of /, percent-encoded characters, unreserved characters, sub-delims (!, $, &, ', (, ), *, +, ,, ;, and =), :, and @.

This means that the specification defines three categories of characters used within a path:

1. Characters which must be considered equivalent regardless of whether or not they are percent encoded. This is the unreserved characters. For example, /%7e%61 and /~a should be considered equivalent URIs. This is important because older implementations may consider ~ to be a reserved character and send %7e which web servers must handle.
2. Characters which must be percent encoded always. For example, non-latin alphabetic characters.
3. Characters which may be present in percent encoded form or verbatim but which may not be normalized during normal URI processing. This includes the remaining characters such as (, !, /, and @. The fact that RFC3986§6.2.2.2 does not say that these may be safely normalized means that the difference between percent-encoded and unencoded variants of these characters may be considered significant by the consumer of the URI.

This last category may be further broken down for JavaScript programmers in terms of what encodeURIComponent() will escape which is everything but !, ', (, ), and *:

encodeURIComponent('/!$^\'()*+,;=:@')
"%2F!%24%5E'()*%2B%2C%3B%3D%3A%40"

And, additionally, I’m sure that the HTML spec also further defines/alters how the URI should be handled.

This patch/PR adds / as a special case targeting ui-router specifically. Instead, it should just fix AngularJS to follow the normalization guidelines which is to only normalize the characters required by RFC3986§2.3.

For example, a function like this could be used instead of decodePath():

/**
 * This takes any percent-encoded character which is listed in
 * RFC3986§2.3 (https://tools.ietf.org/html/rfc3986#section-2.3)
 * and normalizes them to their decoded form. The below is the
 * list of characters:
 *
 * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
 *
 * "-" = %2d
 *
 * "." = %2e
 *
 * "_" = %5f
 *
 * ~ = %7e
 *
 * ALPHA = %41–%5A
 *
 * DIGIT = %30–%39
 *
 * RFC3986§6.2.2.2
 * (https://tools.ietf.org/html/rfc3986#section-6.2.2.2) states
 * that the above characters should be considered equivalent when
 * both encoded and not. Thus, the application is not allowed to
 * treat their escaped and unescaped versions differently. For
 * example, none of those characters may be used as path
 * separators because URI implementations are allowed to normalize
 * them away.
 *
 * To avoid accidentally relying on these characters as path
 * separators, normalize them away.
 */
function normalizePathPercentEncoding(s) {
  // For incoming URIs, we expect all of these characters to already
  // be decoded. This replace should be basically free when the path
  // is already percent encoding normalized.
  return s.replace(/%(?:2d|2e|5f|7e|4[1-9a-f]|5[0-9a]|3[0-9])/gi, decodeURIComponent);
}

It is not the place of AngularJS to decode the entire path. Whatever is handling the path and giving significance to segments—e.g., ui-router which considers the / character to be significant—should be the thing which handles calling decodeURIComponent() on individual components as necessary when extracting the information. A different service consuming $location might decide to use a different character which encodeURIComponent() would percent-encode as a significant character such as & or ; or ?.

[binki]

Additionally, I see that this is only applied for html5Mode. I actually came here because I was having this same exact problem except in hashbang mode.

[binki]

Here’s a demo of this same problem existing in 1.7.3 in hashPrefix() mode: http://embed.plnkr.co/Z3Lh5F2b07uAEVgBpyAd/ . Just type in anything with a slash in it and note that it is interpreted as multiple arguments instead of one even though the link is properly encoded.

[gkalpak]

Thx for the thorough write-up, @binki. You are probably correct.

Unfortunately, changing the behavior of $location is a tricky thing. $location is a very critical part of every AngularJS application and existing code (including popular libraries, such ui-router) are relying on its current (imperfect) behavior. There have been attempts to rectify similar issues in the past, but ended up being reverted, because they apparently broke significantly more usecases than they fixed.

Given that AngularJS has now entered an LTS period, such breaking changes are out of scope.

#

If someone desperately needs this different (more correct) behavior, it is not too difficult to overwrite $location and provide an implementation that matches your requirements. This is even easier than fixing the existing implementation, because one only has to account for their app's needs, not every possible cornercase out there.

[binki]

@gkalpak Thanks for the response and advice. I understand the LTS period issue and compat issues. I was surprised that some of the changes discussed in this thread actually appeared to go out in minor releases during the LTS period :-p.

I was pulled away from working on the project where I was having trouble with $location before I could solve that issue. I think when I get back to it I will try my hand at reimplementing $location. When I was looking into that earlier, it seemed unfortunate that I would need many parts of AngularJS’s core implementation but the module system prevents me from reusing those parts, forcing me to either reimplement everything “for real” or copy most of the core code into my project. I guess I’ll just have to be pragmatic about it, accept that things are the way they are, and start copy-pasting—when I get around to it ;-).

[prakharjaiswal]

We in our application have embedded callback urls in path itself and hence decodePath in non html mode (hash bang) has been causing the trouble.
Tried patching the $$parse and $$compose method using decorator pattern. But, the caveat with this approach is, that the decorators are invoked late and in scenarios where a page (with callback embedded) is refreshed, native implementations of $$parse and $$compose have already been invoked. Hence, the route mismatch.

Any reason why in native implementation; decodePath re-encodes the forward slashes to %2F only for htmlMode and not for hashbang mode?