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

Commit

Permalink
fix($http): do not encode special characters @$:, in params
Browse files Browse the repository at this point in the history
encodeURIComponent is too aggressive and doesn't follow http://www.ietf.org/rfc/rfc3986.txt
with regards to the character set (pchar) allowed in path segments so we need
this test to make sure that we don't over-encode the params and break stuff
like buzz api which uses @self.

This is has already been fixed in `$resource`. This commit fixes it in a same way
for `$http` as well.

BREAKING CHANGE: $http does follow RFC3986 and does not encode special characters
like `$@,:` in params. If your application needs to encode these characters, encode
them manually, before sending the request.
  • Loading branch information
vojtajina committed Feb 14, 2013
1 parent 2a21234 commit 288b69a
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/ng/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -818,8 +818,8 @@ function $HttpProvider() {
if (isObject(v)) {
v = toJson(v);
}
parts.push(encodeURIComponent(key) + '=' +
encodeURIComponent(v));
parts.push(encodeUriQuery(key) + '=' +
encodeUriQuery(v));
});
});
return url + ((url.indexOf('?') == -1) ? '?' : '&') + parts.join('&');
Expand Down
13 changes: 12 additions & 1 deletion test/ng/httpSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ describe('$http', function() {


it('should jsonify objects in params map', inject(function($httpBackend, $http) {
$httpBackend.expect('GET', '/url?a=1&b=%7B%22c%22%3A3%7D').respond('');
$httpBackend.expect('GET', '/url?a=1&b=%7B%22c%22:3%7D').respond('');
$http({url: '/url', params: {a:1, b:{c:3}}, method: 'GET'});
}));

Expand All @@ -153,6 +153,17 @@ describe('$http', function() {
$httpBackend.expect('GET', '/url?a=1&a=2&a=3').respond('');
$http({url: '/url', params: {a: [1,2,3]}, method: 'GET'});
}));


it('should not encode @ in url params', function() {
//encodeURIComponent is too agressive and doesn't follow http://www.ietf.org/rfc/rfc3986.txt
//with regards to the character set (pchar) allowed in path segments
//so we need this test to make sure that we don't over-encode the params and break stuff
//like buzz api which uses @self

$httpBackend.expect('GET', '/Path?!do%26h=g%3Da+h&:bar=$baz@1').respond('');
$http({url: '/Path', params: {':bar': '$baz@1', '!do&h': 'g=a h'}, method: 'GET'});
});
});


Expand Down
4 changes: 1 addition & 3 deletions test/ngResource/resourceSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,7 @@ describe("resource", function() {
});


// In order to get this passed, we need to fix $http - another breaking change,
// so I'm gonna submit that as a separate CL.
xit('should not encode @ in url params', function() {
it('should not encode @ in url params', function() {
//encodeURIComponent is too agressive and doesn't follow http://www.ietf.org/rfc/rfc3986.txt
//with regards to the character set (pchar) allowed in path segments
//so we need this test to make sure that we don't over-encode the params and break stuff like
Expand Down

1 comment on commit 288b69a

@marknadig
Copy link
Contributor

Choose a reason for hiding this comment

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

awesome

Please sign in to comment.