Fix URL encoding bug around AWS v4 auth

AWS v4 auth defined URL encoding algorithm that is used for
generating signature. Mochiweb's utility function convert
space (" ") to plus ("+") but AWS spec says it should be
percent encoded. i.e. "%20".

This commit fixes the encoding bug for resource (URL path) and
query parameters.

cf.
Authenticating Requests by Using the Authorization Header
  (Compute Checksum of the Entire Payload Prior to Transmission)
  - Signature Version 4 - Amazon Simple Storage Service
  http://docs.aws.amazon.com/ja_jp/AmazonS3/latest/API/sig-v4-header-based-auth.html

client_tests/python/boto_tests/boto_test.py

@@ -164,6 +164,17 @@ def test_auth(self):
         conn = self.make_connection(bad_user)
         self.assertRaises(S3ResponseError, conn.get_canonical_user_id)
 
+    def test_auth_weird_query_param(self):
+        bucket = self.conn.create_bucket(self.bucket_name)
+        query_args = 'foo=bar%20baz'
+        response = bucket.connection.make_request('GET', bucket.name, "notfound",
+                                                  query_args=query_args)
+        body = response.read()
+        boto.log.debug(body)
+        if response.status != 404:
+            raise bucket.connection.provider.storage_response_error(
+                response.status, response.reason, body)
+
     def test_create_bucket(self):
         self.conn.create_bucket(self.bucket_name)
         self.assertIn(self.bucket_name,
@@ -317,7 +328,6 @@ def test_list_japanese_key(self):
         for u in uploads:
             self.assertEqual(u.key_name, key_name)
 
-
 def one_kb_string():
     "Return a 1KB string of all a's"
     return ''.join(['a' for _ in xrange(1024)])
@@ -665,14 +675,16 @@ class ObjectMetadataTest(S3ApiVerificationTestBase):
         "Expires": "Tue, 19 Jan 2038 03:14:07 GMT",
         "mtime": "1364742057",
         "UID": "0",
-        "with-hypen": "1"}
+        "with-hypen": "1",
+        "space-in-value": "abc xyz"}
 
     updated_metadata = {
         "Content-Disposition": 'attachment; filename="newname.txt"',
         "Cache-Control": "private",
         "Expires": "Tue, 19 Jan 2038 03:14:07 GMT",
         "mtime": "2222222222",
         "uid": "0",
+        "space-in-value": "ABC XYZ",
         "new-entry": "NEW"}
 
     def test_normal_object_metadata(self):
@@ -708,12 +720,14 @@ def assert_metadata(self, bucket, key_name):
         self.assertEqual(key.get_metadata("mtime"), "1364742057")
         self.assertEqual(key.get_metadata("uid"), "0")
         self.assertEqual(key.get_metadata("with-hypen"), "1")
+        self.assertEqual(key.get_metadata("space-in-value"), "abc xyz")
         # x-amz-meta-* headers should be normalized to lowercase
         self.assertEqual(key.get_metadata("Mtime"), None)
         self.assertEqual(key.get_metadata("MTIME"), None)
         self.assertEqual(key.get_metadata("Uid"), None)
         self.assertEqual(key.get_metadata("UID"), None)
         self.assertEqual(key.get_metadata("With-Hypen"), None)
+        self.assertEqual(key.get_metadata("Space-In-Value"), None)
 
     def change_metadata(self, bucket, key_name):
         key = Key(bucket, key_name)
@@ -730,6 +744,7 @@ def assert_updated_metadata(self, bucket, key_name):
                          'attachment; filename="newname.txt"')
         self.assertEqual(key.cache_control, "private")
         self.assertEqual(key.get_metadata("mtime"), "2222222222")
+        self.assertEqual(key.get_metadata("space-in-value"), "ABC XYZ")
         # removed
         self.assertEqual(key.content_encoding, None)
         self.assertEqual(key.get_metadata("with-hypen"), None)

src/riak_cs_s3_auth.erl

@@ -38,6 +38,14 @@
 -define(QS_KEYID, "AWSAccessKeyId").
 -define(QS_SIGNATURE, "Signature").
 
+-define(PERCENT, 37).  % $\%
+-define(FULLSTOP, 46). % $\.
+-define(QS_SAFE(C), ((C >= $a andalso C =< $z) orelse
+                     (C >= $A andalso C =< $Z) orelse
+                     (C >= $0 andalso C =< $9) orelse
+                     (C =:= ?FULLSTOP orelse C =:= $- orelse C =:= $~ orelse
+                      C =:= $_))).
+
 %% ===================================================================
 %% Public API
 %% ===================================================================
@@ -336,18 +344,39 @@ canonicalize_qs_v4([{K, V}|T], Acc) ->
 %% Because keys and values from raw query string that is URL-encoded at client
 %% side, they should be URL-decoded once and encoded back.
 strict_url_encode_for_qs_value(Value) ->
-    mochiweb_util:quote_plus(mochiweb_util:unquote(Value)).
+    quote_percent(mochiweb_util:unquote(Value)).
 
 %% Force string URL encoding for path part of URL.
 %% Contrary to query part, slashes MUST NOT encoded and left as is.
 strict_url_encode_for_path(Path) ->
     %% Use `binary:split/3' here instead of `string:tokens/2' because
     %% latter drops information about preceding and trailing slashes.
     Tokens = binary:split(list_to_binary(Path), <<"/">>, [global]),
-    EncodedTokens = [mochiweb_util:quote_plus(mochiweb_util:unquote(T)) ||
+    EncodedTokens = [quote_percent(mochiweb_util:unquote(T)) ||
                         T <- Tokens],
     string:join(EncodedTokens, "/").
 
+-spec quote_percent(string()) -> string().
+%% @doc URL safe encoding of the given string, keeping with the strict
+%% specification of AWS S3 because this is used for canonicalizing
+%% resource for signature.
+quote_percent(String) ->
+    quote_percent(String, []).
+
+quote_percent([], Acc) ->
+    lists:reverse(Acc);
+quote_percent([C | Rest], Acc) when ?QS_SAFE(C) ->
+    quote_percent(Rest, [C | Acc]);
+quote_percent([$\s | Rest], Acc) ->
+    quote_percent(Rest, [$0, $2, ?PERCENT | Acc]);
+quote_percent([C | Rest], Acc) ->
+    <<Hi:4, Lo:4>> = <<C>>,
+    quote_percent(Rest, [hexdigit(Lo), hexdigit(Hi), ?PERCENT | Acc]).
+
+hexdigit(C) when C < 10 -> $0 + C;
+hexdigit(C) when C < 16 -> $A + (C - 10).
+
+
 %% ===================================================================
 %% Eunit tests
 %% ===================================================================
@@ -532,4 +561,13 @@ example_unicode_keys() ->
     CalculatedSignature = calculate_signature_v2(KeyData, RD),
     test_fun("example unicode keys test", ExpectedSignature, CalculatedSignature).
 
+quote_percent_test() ->
+    ?assertEqual("", quote_percent("")),
+    ?assertEqual("abc", quote_percent("abc")),
+    ?assertEqual("%20", quote_percent(" ")),
+    ?assertEqual("abc%20%20xyz%20%0A", quote_percent("abc  xyz \n")),
+    ?assertEqual("%24%2A%3F%0A", quote_percent("$*?\n")),
+    ?assertEqual("foo%3B%26%3D", quote_percent("foo;&=")),
+    ok.
+
 -endif.