From de02e53c9461e205ee40ccd4ae50434cbede0b41 Mon Sep 17 00:00:00 2001 From: Andrii Sultanov Date: Tue, 25 Jun 2024 16:16:46 +0100 Subject: [PATCH 1/2] CP-49953: Remove parse_uri, switch to using Uri module instead Signed-off-by: Andrii Sultanov --- ocaml/libs/http-lib/http.ml | 45 --------------------------------- ocaml/libs/http-lib/http.mli | 6 ----- ocaml/libs/http-lib/http_svr.ml | 11 +++++++- 3 files changed, 10 insertions(+), 52 deletions(-) diff --git a/ocaml/libs/http-lib/http.ml b/ocaml/libs/http-lib/http.ml index be2d4c2c0c5..6b92a014512 100644 --- a/ocaml/libs/http-lib/http.ml +++ b/ocaml/libs/http-lib/http.ml @@ -208,15 +208,6 @@ let parse_keyvalpairs xs = ) kvpairs -let parse_uri x = - match Astring.String.cuts ~sep:"?" x with - | [uri] -> - (uri, []) - | [uri; params] -> - (uri, parse_keyvalpairs params) - | _ -> - raise Http_parse_failure - type authorization = Basic of string * string | UnknownAuth of string [@@deriving rpc] @@ -629,42 +620,6 @@ module Request = struct let get_version x = x.version - let of_request_line x = - match Astring.String.fields ~empty:false x with - | [m; uri; version] -> ( - (* Request-Line = Method SP Request-URI SP HTTP-Version CRLF *) - let uri, query = parse_uri uri in - (* strip the "HTTP/" prefix from the version string *) - match Astring.String.cut ~sep:"/" version with - | Some (_, version) -> - { - m= method_t_of_string m - ; frame= false - ; uri - ; query - ; content_length= None - ; transfer_encoding= None - ; accept= None - ; version - ; cookie= [] - ; auth= None - ; task= None - ; subtask_of= None - ; content_type= None - ; host= None - ; user_agent= None - ; close= false - ; additional_headers= [] - ; body= None - ; traceparent= None - } - | None -> - error "Failed to parse: %s" x ; - raise Http_parse_failure - ) - | _ -> - raise Http_parse_failure - let to_string x = let kvpairs x = String.concat "; " (List.map (fun (k, v) -> k ^ "=" ^ v) x) diff --git a/ocaml/libs/http-lib/http.mli b/ocaml/libs/http-lib/http.mli index 84326e38012..0f561391de7 100644 --- a/ocaml/libs/http-lib/http.mli +++ b/ocaml/libs/http-lib/http.mli @@ -119,10 +119,6 @@ module Request : sig val get_version : t -> string (** [get_version t] returns the HTTP protocol version *) - val of_request_line : string -> t - (** [of_request_line l] parses [l] of the form "METHOD HTTP/VERSION" and - returns the corresponding [t] *) - val to_string : t -> string (** [to_string t] returns a short string summarising [t] *) @@ -176,8 +172,6 @@ end val authorization_of_string : string -> authorization -val parse_uri : string -> string * (string * string) list - val http_403_forbidden : ?version:string -> unit -> string list val http_200_ok : ?version:string -> ?keep_alive:bool -> unit -> string list diff --git a/ocaml/libs/http-lib/http_svr.ml b/ocaml/libs/http-lib/http_svr.ml index c824277e5be..2950bb3f79b 100644 --- a/ocaml/libs/http-lib/http_svr.ml +++ b/ocaml/libs/http-lib/http_svr.ml @@ -359,6 +359,12 @@ let request_of_bio_exn ~proxy_seen ~read_timeout ~total_timeout ~max_length bio proxy |> Option.fold ~none:[] ~some:(fun p -> [("STUNNEL_PROXY", p)]) in let open Http.Request in + (* Below transformation only keeps one value per key, whereas + a fully compliant implementation following Uri's interface + would operate on list of values for each key instead *) + let kvlist_flatten ls = + List.map (function k, v :: _ -> (k, v) | k, [] -> (k, "")) ls + in let request = Astring.String.cuts ~sep:"\n" headers |> List.fold_left @@ -367,7 +373,10 @@ let request_of_bio_exn ~proxy_seen ~read_timeout ~total_timeout ~max_length bio match Astring.String.fields ~empty:false header with | [meth; uri; version] -> (* Request-Line = Method SP Request-URI SP HTTP-Version CRLF *) - let uri, query = Http.parse_uri uri in + let uri_t = Uri.of_string uri in + if uri_t = Uri.empty then raise Http_parse_failure ; + let uri = Uri.path uri_t in + let query = Uri.query uri_t |> kvlist_flatten in let m = Http.method_t_of_string meth in let version = let x = String.trim version in From e53ce67ad6fe226008f1877bd346461b372a110d Mon Sep 17 00:00:00 2001 From: Andrii Sultanov Date: Wed, 3 Jul 2024 13:06:38 +0100 Subject: [PATCH 2/2] Fix a bug noticed by a quicktest run Introduces percent-decoding back - in the past we used to do urlencode in parse_uri instead. Signed-off-by: Andrii Sultanov --- ocaml/libs/http-lib/http_svr.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocaml/libs/http-lib/http_svr.ml b/ocaml/libs/http-lib/http_svr.ml index 2950bb3f79b..c27f59a7949 100644 --- a/ocaml/libs/http-lib/http_svr.ml +++ b/ocaml/libs/http-lib/http_svr.ml @@ -375,7 +375,7 @@ let request_of_bio_exn ~proxy_seen ~read_timeout ~total_timeout ~max_length bio (* Request-Line = Method SP Request-URI SP HTTP-Version CRLF *) let uri_t = Uri.of_string uri in if uri_t = Uri.empty then raise Http_parse_failure ; - let uri = Uri.path uri_t in + let uri = Uri.path uri_t |> Uri.pct_decode in let query = Uri.query uri_t |> kvlist_flatten in let m = Http.method_t_of_string meth in let version =