encode query params correctly but not too agressively

4 files changed, 65 insertions, 12 deletions

diff --git a/src/Angular.js b/src/Angular.js
index 9a607ba3..e297905a 100644
--- a/src/Angular.js
+++ b/src/Angular.js
@@ -811,20 +811,44 @@ function toKeyValue(obj) {
 
 /**
  * we need our custom mehtod because encodeURIComponent is too agressive and doesn't follow
- * http://www.ietf.org/rfc/rfc2396.txt with regards to the character set (pchar) allowed in path
- * segments
+ * http://www.ietf.org/rfc/rfc3986.txt with regards to the character set (pchar) allowed in path
+ * segments:
+ *    segment       = *pchar
+ *    pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
+ *    pct-encoded   = "%" HEXDIG HEXDIG
+ *    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
+ *    sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
+ *                     / "*" / "+" / "," / ";" / "="
  */
 function encodeUriSegment(val) {
+  return encodeUriQuery(val, true).
+             replace(/%26/gi, '&').
+             replace(/%3D/gi, '=').
+             replace(/%2B/gi, '+');
+}
+
+
+/**
+ * This method is intended for encoding *key* or *value* parts of query component. We need a custom
+ * method becuase encodeURIComponent is too agressive and encodes stuff that doesn't have to be
+ * encoded per http://tools.ietf.org/html/rfc3986:
+ *    query       = *( pchar / "/" / "?" )
+ *    pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
+ *    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
+ *    pct-encoded   = "%" HEXDIG HEXDIG
+ *    sub-delims    = "!" / "$" / "&" / "'" / "(" / ")"
+ *                     / "*" / "+" / "," / ";" / "="
+ */
+function encodeUriQuery(val, pctEncodeSpaces) {
   return encodeURIComponent(val).
              replace(/%40/gi, '@').
              replace(/%3A/gi, ':').
-             replace(/%26/gi, '&').
-             replace(/%3D/gi, '=').
-             replace(/%2B/gi, '+').
              replace(/%24/g, '$').
-             replace(/%2C/gi, ',');
+             replace(/%2C/gi, ',').
+             replace((pctEncodeSpaces ? null : /%20/g), '+');
 }
 
+
 /**
  * @workInProgress
  * @ngdoc directive
diff --git a/src/Resource.js b/src/Resource.js
index ade110db..e2cd5a0e 100644
--- a/src/Resource.js
+++ b/src/Resource.js
@@ -26,7 +26,7 @@ Route.prototype = {
     var query = [];
     forEachSorted(params, function(value, key){
       if (!self.urlParams[key]) {
-        query.push(encodeURIComponent(key) + '=' + encodeURIComponent(value));
+        query.push(encodeUriQuery(key) + '=' + encodeUriQuery(value));
       }
     });
     url = url.replace(/\/*$/, '');
diff --git a/test/AngularSpec.js b/test/AngularSpec.js
index 61dc7cfc..9b3357e5 100644
--- a/test/AngularSpec.js
+++ b/test/AngularSpec.js
@@ -55,7 +55,6 @@ describe('angular', function(){
       expect(copy(123)).toEqual(123);
       expect(copy([{key:null}])).toEqual([{key:null}]);
     });
-
   });
 
   describe('equals', function(){
@@ -170,7 +169,7 @@ describe('angular', function(){
 
 
   describe('encodeUriSegment', function() {
-    it('should correctly encode uri segment and not encode chars defined as pchar set in rfc2396',
+    it('should correctly encode uri segment and not encode chars defined as pchar set in rfc3986',
         function() {
       //don't encode alphanum
       expect(encodeUriSegment('asdf1234asdf')).
@@ -191,6 +190,36 @@ describe('angular', function(){
   });
 
 
+  describe('encodeUriQuery', function() {
+    it('should correctly encode uri query and not encode chars defined as pchar set in rfc3986',
+        function() {
+      //don't encode alphanum
+      expect(encodeUriQuery('asdf1234asdf')).
+        toEqual('asdf1234asdf');
+
+      //don't encode unreserved
+      expect(encodeUriQuery("-_.!~*'() -_.!~*'()")).
+        toEqual("-_.!~*'()+-_.!~*'()");
+
+      //don't encode the rest of pchar
+      expect(encodeUriQuery(':@$, :@$,')).
+        toEqual(':@$,+:@$,');
+
+      //encode '&', ';', '=', '+', and '#'
+      expect(encodeUriQuery('&;=+# &;=+#')).
+        toEqual('%26%3B%3D%2B%23+%26%3B%3D%2B%23');
+
+      //encode ' ' as '+'
+      expect(encodeUriQuery('  ')).
+        toEqual('++');
+
+      //encode ' ' as '%20' when a flag is used
+      expect(encodeUriQuery('  ', true)).
+        toEqual('%20%20');
+    });
+  });
+
+
   describe ('rngScript', function() {
     it('should match angular.js', function() {
       expect('angular.js'.match(rngScript)).not.toBeNull();
diff --git a/test/ResourceSpec.js b/test/ResourceSpec.js
index 663b163d..e055beab 100644
--- a/test/ResourceSpec.js
+++ b/test/ResourceSpec.js
@@ -49,14 +49,14 @@ describe("resource", function() {
   });
 
   it('should not encode @ in url params', function() {
-    //encodeURIComponent is too agressive and doesn't follow http://www.ietf.org/rfc/rfc2396.txt
+    //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
 
     var R = resource.route('/Path/:a');
-    xhr.expectGET('/Path/doh@foo?bar=baz%401').respond({});
-    R.get({a: 'doh@foo', bar: 'baz@1'});
+    xhr.expectGET('/Path/doh@fo%20o?!do%26h=g%3Da+h&:bar=$baz@1').respond({});
+    R.get({a: 'doh@fo o', ':bar': '$baz@1', '!do&h': 'g=a h'});
   });
 
   it('should encode & in url params', function() {