Send paths on the wire in the form they were signed

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 *)

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
 

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