diff --git a/client_tests/python/boto_tests/boto_test.py b/client_tests/python/boto_tests/boto_test.py index e6a93b05f..5b43194c0 100755 --- a/client_tests/python/boto_tests/boto_test.py +++ b/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,7 +675,8 @@ 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"', @@ -673,6 +684,7 @@ class ObjectMetadataTest(S3ApiVerificationTestBase): "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) diff --git a/src/riak_cs_s3_auth.erl b/src/riak_cs_s3_auth.erl index f6e1da48f..8a7fa215e 100644 --- a/src/riak_cs_s3_auth.erl +++ b/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,7 +344,7 @@ 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. @@ -344,10 +352,31 @@ 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) -> + <> = <>, + 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.