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

ngResource: '$' in pamater values are not escaped properly #6003

Closed
tam4s opened this issue Jan 27, 2014 · 1 comment
Closed

ngResource: '$' in pamater values are not escaped properly #6003

tam4s opened this issue Jan 27, 2014 · 1 comment

Comments

@tam4s
Copy link

tam4s commented Jan 27, 2014

1 liner demonstration in a browser JS console:

angular.injector(['ngResource']).get('$resource')('/search/:str').get({'str': 'hello$'})
GET http://docs.angularjs.org/search/hello$1 404 (Not Found)

Notice the extra '1' in the URL.

caitp added a commit to caitp/angular.js that referenced this issue Jan 27, 2014
…ncoding URI

Previously, if a URL parameter value included a $, it would replace the dollar sign with a literal
'$1' for mysterious reasons. Using a function rather than a replacement string circumvents this
behaviour and produces a more expected result.

Closes angular#6003
@caitp
Copy link
Contributor

caitp commented Jan 27, 2014

Interesting, I've taken a stab at correcting this, would you care to try my patch and see if it introduces any regressions not caught by the tests?

@ghost ghost assigned vojtajina Jan 28, 2014
caitp added a commit to caitp/angular.js that referenced this issue Feb 3, 2014
…ncoding URI

Previously, if a URL parameter value included a $, it would replace the dollar sign with a literal
'$1' for mysterious reasons. Using a function rather than a replacement string circumvents this
behaviour and produces a more expected result.

Closes angular#6003
caitp added a commit to caitp/angular.js that referenced this issue Feb 3, 2014
…ncoding URI

Previously, if a URL parameter value included a $, it would replace the dollar sign with a literal
'$1' for mysterious reasons. Using a function rather than a replacement string circumvents this
behaviour and produces a more expected result.

Closes angular#6003
@caitp caitp closed this as completed in ce1f1f9 Feb 3, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants