diff --git a/aws-s3/http.ml b/aws-s3/http.ml index dac2724..0d0e81b 100644 --- a/aws-s3/http.ml +++ b/aws-s3/http.ml @@ -8,6 +8,31 @@ let log fmt = match debug with type meth = [ `DELETE | `GET | `HEAD | `POST | `PUT ] +(* The request target must reach the wire exactly as it was signed, so encode it + here rather than routing it through [Uri], which decodes reserved characters + ("%2B" back to "+") and breaks the signature. *) +let request_target ~path ~query = + let encoded_query = + query + |> List.map ~f:(fun (k, v) -> + sprintf "%s=%s" + (Uri.pct_encode ~component:`Userinfo k) + (Uri.pct_encode ~component:`Userinfo v)) + |> String.concat ~sep:"&" + in + let path = Util.encode_string path in + if encoded_query = "" then path else sprintf "%s?%s" path encoded_query + +let%test "request_target keeps '+' percent-encoded" = + request_target ~path:"/a+b" ~query:[] = "/a%2Bb" + +let%test "request_target path matches the signed encoding" = + let path = "/tsync/Audiosocket Info Sheet + Tax Docs.pdf" in + request_target ~path ~query:[] = Util.encode_string path + +let%test "request_target appends the encoded query" = + request_target ~path:"/k" ~query:[("prefix", "a+b")] = "/k?prefix=a%2Bb" + module Make(Io : Types.Io) = struct module Body = Body.Make(Io) open Io @@ -49,10 +74,7 @@ module Make(Io : Types.Io) = struct | true -> Headers.add ~key:"Expect" ~value:"100-continue" headers | false -> headers in - let path_with_params = - let query = List.map ~f:(fun (k, v) -> k, [v]) query in - Uri.make ~path ~query () |> Uri.to_string - in + let path_with_params = request_target ~path ~query in let header = sprintf "%s %s HTTP/1.1\r\n" (string_of_method meth) path_with_params in Pipe.write writer header >>= fun () -> (* Write all headers *) diff --git a/aws-s3/http.mli b/aws-s3/http.mli index 957b7ff..09da00e 100644 --- a/aws-s3/http.mli +++ b/aws-s3/http.mli @@ -1,6 +1,8 @@ (**/**) type meth = [ `DELETE | `GET | `HEAD | `POST | `PUT ] +val request_target : path:string -> query:(string * string) list -> string + module Make : functor(Io: Types.Io) -> sig open Io diff --git a/aws-s3/util.ml b/aws-s3/util.ml index 99c37d8..22e4d30 100644 --- a/aws-s3/util.ml +++ b/aws-s3/util.ml @@ -46,19 +46,27 @@ let encode_string s = | 'A' .. 'Z' | '0' .. '9' | '_' | '-' | '~' | '.' | '/' -> Buffer.add_char buf c - | '%' -> - (* Sigh. Annoying we're expecting already escaped strings so ignore the escapes *) - begin - let is_hex = function - | 'a' .. 'f' | 'A' .. 'F' | '0' .. '9' -> true - | _ -> false - in - if (i + 2) < n then - if is_hex(String.get s (i+1)) && is_hex(String.get s (i+2)) then - Buffer.add_char buf c - else - Buffer.add_string buf "%25" - end - | _ -> Buffer.add_string buf (Printf.sprintf "%%%X" (Char.code c)) + (* Callers build their path as "/bucket/key" from a raw key, so a '%' here + belongs to the name rather than to an escape already applied. *) + | '%' -> Buffer.add_string buf "%25" + | _ -> Buffer.add_string buf (Printf.sprintf "%%%02X" (Char.code c)) done; Buffer.contents buf + +let%test "encode_string escapes a literal percent" = + encode_string "/b/percent-%2F-key" = "/b/percent-%252F-key" + +let%test "encode_string escapes a plus" = encode_string "/b/a+b" = "/b/a%2Bb" + +let%test "encode_string keeps a trailing percent" = + encode_string "/b/trailing%" = "/b/trailing%25" + +let%test "encode_string leaves the unreserved set alone" = + encode_string "/b/aZ0_-~./x" = "/b/aZ0_-~./x" + +let%test "encode_string pads a low byte" = + encode_string "/b/\n" = "/b/%0A" + +let%test "encode_string round-trips through a decoder" = + let name = "percent-%2F-key" in + Uri.pct_decode (encode_string ("/b/" ^ name)) = "/b/" ^ name