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

fix(ngRoute): remove extra decodeURIComponent #6327

Closed
wants to merge 1 commit into from

Conversation

mllrjb
Copy link
Contributor

@mllrjb mllrjb commented Feb 18, 2014

Since $location.$$path is already decoded, doing an extra decodeURIComponent is both unnecessary and can cause problems. Specifically, if the path originally included an encoded % (aka %25), then ngRoute will throw "URIError: URI malformed".

Closes #6326

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#6327)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@@ -465,9 +465,7 @@ function $RouteProvider(){
for (var i = 1, len = m.length; i < len; ++i) {
var key = keys[i - 1];

var val = 'string' == typeof m[i]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This appears to have just been a mistake in the copy/paste from Express.js's routing logic.

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@mllrjb mllrjb added cla: no and removed cla: yes labels Feb 19, 2014
Since `$location.$$path` is already decoded, doing an extra `decodeURIComponent` is both unnecessary and can cause problems. Specifically, if the path originally included an encoded `%` (aka `%25`), then ngRoute will throw "URIError: URI malformed".

Closes angular#6326
@mllrjb mllrjb closed this Feb 20, 2014
@mllrjb mllrjb reopened this Feb 20, 2014
@mary-poppins
Copy link

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@mllrjb mllrjb added cla: yes and removed cla: no labels Feb 20, 2014
@ashleygwilliams ashleygwilliams added this to the Backlog milestone Feb 22, 2014
@mllrjb
Copy link
Contributor Author

mllrjb commented Feb 27, 2014

Can I get some feedback or an ETA? This adversely effects an application I'm developing.

@titoBouzout
Copy link

The book said: Do not decode what you don't encode. This is major issue. It breaks internationalization, it even breaks common peculiar things, imagine Computers/GNU%2FLinux/Something/Else
Angular incorrectly reports and also redirects(!?) to: Computers/GNU/Linux/Something/Else ..

There should be at least a way to get the pure hash, problem is that if its redirecting that hash is also lost. I've had to remove some "decodeURIComponent" from angular code, and I'm not even sure if I'm introducing security issues, because as we know, encodeURIComponent is a way to protect our applications from malicious data..

err., I'm not sure if exactly this issue, is the problem of what I'm talking about, but is very related!

@IgorMinar
Copy link
Contributor

this looks valid to me.

@petebacondarwin can you double-check the test (maybe add some assertion?) and get this in please?

@IgorMinar IgorMinar modified the milestones: 1.3.0-beta.16, Backlog Jul 15, 2014
@petebacondarwin
Copy link
Contributor

Will do
On 15 Jul 2014 07:27, "Igor Minar" [email protected] wrote:

this looks valid to me.

@petebacondarwin https://github.com/petebacondarwin can you
double-check the test (maybe add some assertion?) and get this in please?


Reply to this email directly or view it on GitHub
#6327 (comment).

petebacondarwin pushed a commit that referenced this pull request Jul 15, 2014
Since `$location.$$path` is already decoded, doing an extra `decodeURIComponent` is both unnecessary
and can cause problems. Specifically, if the path originally includes an encoded `%` (aka `%25`),
then ngRoute will throw "URIError: URI malformed".

Closes #6326
Closes #6327
ckknight pushed a commit to ckknight/angular.js that referenced this pull request Jul 16, 2014
Since `$location.$$path` is already decoded, doing an extra `decodeURIComponent` is both unnecessary
and can cause problems. Specifically, if the path originally includes an encoded `%` (aka `%25`),
then ngRoute will throw "URIError: URI malformed".

Closes angular#6326
Closes angular#6327
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.

ngRoute doesn't support "%" in URLs, throws URI malformed
7 participants