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

Potential Bug with URL Hash being decoded (Tested on Angular 1.4.2 & 1.4.7) #13245

Open
dgwaldo opened this issue Nov 4, 2015 · 5 comments
Open

Comments

@dgwaldo
Copy link

dgwaldo commented Nov 4, 2015

After browsing the code and reading http://tools.ietf.org/html/rfc3986#section-2.4 I'm still not sure if this is intended behavior. This is happening for us in one specific instance where we have Angular being used by a component on our page. A 3rd party script sets window.location.hash, and Angular's $locationWatch picks up the url and the url is decoded... When the 3rd party js script grabs the window.location.hash value, its now decoded and the third party script expects it to be encoded still which creates a bug. The reason it occurs in this case is that the third party script is splitting on the "/" character, so 942520H PROCAT SE KOHLER CV680 W/52 SIDE DISCHARGE gets encoded to /s/TPI/942502G_PROCAT_26HP_KAW_W%2F%2F61_SIDE_DISCHARGE//1
Angular modifies it to /s/TPI/942502G_PROCAT_26HP_KAW_W//61_SIDE_DISCHARGE//1. Which is put into json to send over the wire... So the decoding in the hash breaks that process...

Here is an example setting the window.location.hash = "%27" notice that it gets decoded...

5a84f580-821e-11e5-8df0-555485426a96

From what I could glean from the spec this shouldn't happen...

"The characters slash ("/") and question mark ("?") are allowed to
represent data within the fragment identifier. Beware that some
older, erroneous implementations may not handle this data correctly
when it is used as the base URI for relative references (Section
5.1)."

I did find this in the source... looks like I'm hitting the or of the else, seems to happen across all browsers ;)

} else {
// - reloadLocation is needed as browsers don't allow to read out
// the new location.href if a reload happened.
// - the replacement is a workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=407172
return reloadLocation || location.href.replace(/%27/g,"'");
}
@dgwaldo
Copy link
Author

dgwaldo commented Nov 4, 2015

Just found this issue, looks related. #1388

@veneliniliev
Copy link

and i have same problem :(

@petebacondarwin
Copy link
Contributor

@dgwaldo - the code that you point to actually replaces incorrectly encoded single quotes %2F with real quotes ', which is not the cause of your problem...

@petebacondarwin
Copy link
Contributor

The actual line of code that makes the change is https://github.com/angular/angular.js/blob/v1.4.7/src/ng/location.js#L40

locationObj.$$path = decodeURIComponent(prefixed && match.pathname.charAt(0) === '/' ?
      match.pathname.substring(1) : match.pathname);

We are passing in 'abc%2F' as match.pathname but it is setting locationObj.$$path to 'abc/'.

So the problem is with the use of decodeURIComponent

@petebacondarwin
Copy link
Contributor

OK, I think this is a bug we can fix... Having a play right now

@petebacondarwin petebacondarwin added this to the 1.5.x - migration-facilitation milestone Jan 25, 2016
@petebacondarwin petebacondarwin self-assigned this Jan 25, 2016
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Jan 25, 2016
…racters

If the user has purposefully encoded forward slashes into their $location path
(i.e. the bit after the hashbang), then we should not forceably decode it,
since if you want forward slashes you are allowed to place them in an
unencoded form straight into a hash segment of a URL

Closes angular#13245
@Narretz Narretz modified the milestones: 1.5.x, 1.7.x Apr 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants