From 115858467f02f4d90d474475c0636de54727a3b8 Mon Sep 17 00:00:00 2001 From: Ted Burghart Date: Sat, 4 Jun 2016 06:02:13 -0400 Subject: [PATCH 1/3] display upload id (cherry picked from commit 5fdad1110657c22a8de68a4cbf16ca3a62b03f34) --- riak_test/tests/mp_upload_test.erl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/riak_test/tests/mp_upload_test.erl b/riak_test/tests/mp_upload_test.erl index f3c6b18e7..c74de855e 100644 --- a/riak_test/tests/mp_upload_test.erl +++ b/riak_test/tests/mp_upload_test.erl @@ -1,6 +1,6 @@ -%% --------------------------------------------------------------------- +%% ------------------------------------------------------------------- %% -%% Copyright (c) 2007-2013 Basho Technologies, Inc. All Rights Reserved. +%% Copyright (c) 2007-2016 Basho Technologies, Inc. %% %% This file is provided to you under the Apache License, %% Version 2.0 (the "License"); you may not use this file @@ -16,7 +16,7 @@ %% specific language governing permissions and limitations %% under the License. %% -%% --------------------------------------------------------------------- +%% ------------------------------------------------------------------- -module(mp_upload_test). @@ -154,6 +154,7 @@ aborted_upload_test_case(Bucket, Key, Config) -> lager:info("Initiating multipart upload"), InitUploadRes = erlcloud_s3_multipart:initiate_upload(Bucket, Key, [], [], Config), UploadId = erlcloud_s3_multipart:upload_id(InitUploadRes), + lager:info("Upload ID: ~p", [UploadId]), %% Verify the upload id is in list_uploads results and %% that the bucket information is correct @@ -216,6 +217,7 @@ basic_upload_test_case(Bucket, Key, Config) -> lager:info("Initiating multipart upload"), InitUploadRes = erlcloud_s3_multipart:initiate_upload(Bucket, Key, [], [], Config), UploadId = erlcloud_s3_multipart:upload_id(InitUploadRes), + lager:info("Upload ID: ~p", [UploadId]), %% Verify the upload id is in list_uploads results and %% that the bucket information is correct @@ -274,6 +276,7 @@ parts_too_small_test_case(Bucket, Key, Config) -> lager:info("Initiating multipart upload (bad)"), InitUploadRes = erlcloud_s3_multipart:initiate_upload(Bucket, Key, [], [], Config), UploadId = erlcloud_s3_multipart:upload_id(InitUploadRes), + lager:info("Upload ID: ~p", [UploadId]), lager:info("Uploading parts (bad)"), EtagList = upload_and_assert_parts(Bucket, From d95f05e7917f40055755a4752e77b5e2f752bc0e Mon Sep 17 00:00:00 2001 From: Ted Burghart Date: Sat, 4 Jun 2016 08:24:55 -0400 Subject: [PATCH 2/3] corrections to RCS-349/RCS-350 fix (cherry picked from commit bb3d2043c9e993d5fa0f3d9cf73494be0cc1caec) --- src/base64url.erl | 154 ++++++++++++++++++++++++++++++------------ src/riak_cs_utils.erl | 6 +- 2 files changed, 112 insertions(+), 48 deletions(-) diff --git a/src/base64url.erl b/src/base64url.erl index 6018d3662..4bca74e67 100644 --- a/src/base64url.erl +++ b/src/base64url.erl @@ -1,6 +1,6 @@ -%% --------------------------------------------------------------------- +%% ------------------------------------------------------------------- %% -%% Copyright (c) 2007-2013 Basho Technologies, Inc. All Rights Reserved. +%% Copyright (c) 2007-2016 Basho Technologies, Inc. %% %% This file is provided to you under the Apache License, %% Version 2.0 (the "License"); you may not use this file @@ -16,7 +16,7 @@ %% specific language governing permissions and limitations %% under the License. %% -%% --------------------------------------------------------------------- +%% ------------------------------------------------------------------- %% @doc base64url is a wrapper around the base64 module to produce %% base64-compatible encodings that are URL safe. @@ -27,46 +27,66 @@ -module(base64url). --include_lib("eunit/include/eunit.hrl"). - -export([decode/1, decode_to_string/1, encode/1, encode_to_string/1]). +-ifdef(TEST). +-compile(export_all). +-include_lib("eunit/include/eunit.hrl"). +-endif. + decode(Base64url) -> - base64:decode(amend_equal(urldecode(Base64url))). + base64:decode(append_equals(urldecode(Base64url))). decode_to_string(Base64url) -> - base64:decode_to_string(amend_equal(urldecode(Base64url))). + base64:decode_to_string(append_equals(urldecode(Base64url))). encode(Data) -> - urlencode(strip_equal(base64:encode(Data))). + urlencode(strip_equals(base64:encode(Data))). encode_to_string(Data) -> - urlencode(strip_equal(base64:encode_to_string(Data))). - --spec strip_equal(binary() | string()) -> binary()|string(). -strip_equal(Encoded) when is_list(Encoded) -> - hd(string:tokens(Encoded, "=")); -strip_equal(Encoded) when is_binary(Encoded) -> - LCS = binary:longest_common_suffix([Encoded, <<"===">>]), - binary:part(Encoded, 0, byte_size(Encoded)-LCS). - -%% @doc complements '=' if it doesn't have 4*n length --spec amend_equal(binary()|string()) -> binary()|string(). -amend_equal(Encoded) when is_list(Encoded) -> - Suffix = case length(Encoded) rem 4 of - 0 -> ""; - L -> [$=||_<-lists:seq(1,L)] - end, - lists:flatten([Encoded, Suffix]); -amend_equal(Bin) when is_binary(Bin) -> + urlencode(strip_equals(base64:encode_to_string(Data))). + +-spec strip_equals(binary() | string()) -> binary()|string(). +%% @private Strip off trailing '=' characters. +strip_equals(Str) when is_list(Str) -> + string:strip(Str, right, $=); +strip_equals(Bin) when is_binary(Bin) -> + LCS = binary:longest_common_suffix([Bin, <<"===">>]), + binary:part(Bin, 0, byte_size(Bin)-LCS). + +-spec append_equals(binary()|string()) -> binary()|string(). +%% @private Append trailing '=' characters to make result legal Base64 length. +%% The most common use case will be a B64-encoded UUID, requiring the addition +%% of 2 characters, so that's the first check. We assume 0 and 3 are equally +%% likely. +%% Because B64 encoding spans all bytes across two characters, the remainder +%% of (length / 4) can never be 1 with a valid encoding, so we throw a badarg +%% with the argument here rather than letting it percolate up from stdlib with +%% no worthwhile information. +append_equals(Str) when is_list(Str) -> + case length(Str) rem 4 of + 2 -> + Str ++ "=="; + 0 -> + Str; + 3 -> + Str ++ "="; + 1 -> + erlang:error(badarg, [Str]) + end; +append_equals(Bin) when is_binary(Bin) -> case byte_size(Bin) rem 4 of - 0 -> Bin; - 1 -> <>; - 2 -> <>; - 3 -> <>; + 1 -> + erlang:error(badarg, [Bin]) end. urlencode(Base64) when is_list(Base64) -> @@ -88,20 +108,64 @@ urldecode_digit($-) -> $+; urldecode_digit(D) -> D. -ifdef(TEST). -equal_strip_amend_test() -> - %% TODO: rewrite this with EQC - _ = [begin - UUID = druuid:v4(), - Encoded = base64url:encode(UUID), - ?assertEqual(nomatch, binary:match(Encoded, [<<"=">>, <<"+">>, <<"/">>])), - ?assertEqual(UUID, base64url:decode(Encoded)) - end || _<- lists:seq(1, 1024)], - _ = [begin - UUID = druuid:v4(), - Encoded = base64url:encode_to_string(UUID), - ?assertEqual(nomatch, re:run(Encoded, "(=\\+\\/)")), - ?assertEqual(UUID, base64url:decode(Encoded)) - end || _<- lists:seq(1, 1024)], - ok. + +illegal_char_REs() -> + BinRE = binary:compile_pattern([<<"+">>, <<"/">>, <<"=">>]), + {ok, StrRE} = re:compile("[\\+/=]"), + {BinRE, StrRE}. + +encode_decode_test() -> + crypto:start(), + % Make sure Rand is at least twice as long as the highest count, but + % because we're depleting the entropy pool don't go overboard! + RandLen = 2050, + % crypto:rand:bytes/1 would be fine for us, but it's gone in OTP-19, + % so use the good stuff rather than bothering with a version check. + BinRand = crypto:strong_rand_bytes(RandLen), + % Swap halves so the binary and string tests don't use the same sequences. + HalfLen = (RandLen div 2), + RandLo = binary_part(BinRand, 0, HalfLen), + RandHi = binary_part(BinRand, HalfLen, HalfLen), + StrRand = << RandHi/binary, RandLo/binary >>, + {BinRE, StrRE} = illegal_char_REs(), + test_encode_decode_uuid(256, BinRE, StrRE), + test_encode_decode_binary(1024, BinRE, StrRE, BinRand), + test_encode_decode_string(384, BinRE, StrRE, StrRand). + +test_encode_decode_uuid(0, _, _) -> + ok; +test_encode_decode_uuid(Count, BinRE, StrRE) -> + test_encode_decode(druuid:v4(), BinRE, StrRE), + test_encode_decode_uuid((Count - 1), BinRE, StrRE). + +test_encode_decode_binary(0, _, _, _) -> + ok; +test_encode_decode_binary(Count, BinRE, StrRE, Rand) -> + test_encode_decode(binary:part(Rand, Count, Count), BinRE, StrRE), + test_encode_decode_binary((Count - 1), BinRE, StrRE, Rand). + +test_encode_decode_string(0, _, _, _) -> + ok; +test_encode_decode_string(Count, BinRE, StrRE, Rand) -> + test_encode_decode(binary:bin_to_list(Rand, Count, Count), BinRE, StrRE), + test_encode_decode_string((Count - 1), BinRE, StrRE, Rand). + +test_encode_decode(Data, BinRE, StrRE) when is_binary(Data) -> + EncBin = base64url:encode(Data), + EncStr = base64url:encode_to_string(Data), + ?assertEqual(EncStr, binary_to_list(EncBin)), + ?assertEqual(nomatch, binary:match(EncBin, BinRE)), + ?assertEqual(nomatch, re:run(EncStr, StrRE)), + ?assertEqual(Data, base64url:decode(EncBin)), + ?assertEqual(Data, base64url:decode(EncStr)); + +test_encode_decode(Data, BinRE, StrRE) when is_list(Data) -> + EncBin = base64url:encode(Data), + EncStr = base64url:encode_to_string(Data), + ?assertEqual(EncStr, binary_to_list(EncBin)), + ?assertEqual(nomatch, binary:match(EncBin, BinRE)), + ?assertEqual(nomatch, re:run(EncStr, StrRE)), + ?assertEqual(Data, base64url:decode_to_string(EncBin)), + ?assertEqual(Data, base64url:decode_to_string(EncStr)). -endif. diff --git a/src/riak_cs_utils.erl b/src/riak_cs_utils.erl index 3a551d726..dd67208ec 100644 --- a/src/riak_cs_utils.erl +++ b/src/riak_cs_utils.erl @@ -1,6 +1,6 @@ -%% --------------------------------------------------------------------- +%% ------------------------------------------------------------------- %% -%% Copyright (c) 2007-2014 Basho Technologies, Inc. All Rights Reserved. +%% Copyright (c) 2007-2016 Basho Technologies, Inc. %% %% This file is provided to you under the Apache License, %% Version 2.0 (the "License"); you may not use this file @@ -16,7 +16,7 @@ %% specific language governing permissions and limitations %% under the License. %% -%% --------------------------------------------------------------------- +%% ------------------------------------------------------------------- %% @doc riak_cs utility functions From f69aea81f1ad0a660602ea69cd76543fd40899a8 Mon Sep 17 00:00:00 2001 From: Ted Burghart Date: Sat, 4 Jun 2016 09:41:08 -0400 Subject: [PATCH 3/3] updated RNs to reference PR 1316 --- RELEASE-NOTES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 9e342a1f7..bebe2422e 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -17,7 +17,7 @@ During the update to 2.1.2, a '==' omitted upload ID might be passed to a Riak C ##Changes * Due to a recent [Product Advisory](http://docs.basho.com/riak/latest/community/product-advisories/codeinjectioninitfiles/), node_package was bumped to version 3.0.0 to prevent a potential code injection on the riak init file. [[Issue 1297](https://github.com/basho/riak_cs/issues/1297), [PR 1306](https://github.com/basho/riak_cs/pull/1306), & [PR 109](https://github.com/basho/stanchion/pull/109)] -* Multipart upload IDs no longer contain trailing '=' characters, which caused trouble for some clients. This change also makes upload IDs URL-safe. This commit removes several unused functions. [[PR 1293](https://github.com/basho/riak_cs/pull/1293)] +* Multipart upload IDs no longer contain trailing '=' characters, which caused trouble for some clients. This change also makes upload IDs URL-safe. [[PR 1316](https://github.com/basho/riak_cs/pull/1316)] * When Riak is unavailable due to network partition or node being offline, a 500 error is returned. [[PR 1298](https://github.com/basho/riak_cs/pull/1298)] ## Bugs Fixed