Skip to content

Conversation

last-genius
Copy link
Contributor

@last-genius last-genius commented Jul 4, 2024

Follows on from #5726.

Introduces a fix to a bug noticed by a failing quicktest (CA-395184):

diff --git a/ocaml/libs/http-lib/http_svr.ml b/ocaml/libs/http-lib/http_svr.ml
index 2950bb3f7..c27f59a79 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 =

Query doesn't need to be percent-decoded, the problem is in Uri.path:

utop # Uri.of_string "/foo?x=<>`&" |> Uri.query;;
- : (string * string list) list = [("x", ["<>`"]); ("", [])]
utop # Uri.of_string "/foo<>`&?x=<>`&" |> Uri.path;;
- : string = "/foo%3C%3E%60&"
utop # Uri.of_string "/foo<>`&?x=<>`&" |> Uri.path |> Uri.pct_decode;;
- : string = "/foo<>`&"

=====

This change passed the BST+BVT test suites

Andrii Sultanov added 2 commits June 26, 2024 14:17
Introduces percent-decoding back - in the past we used to do
urlencode in parse_uri instead.

Signed-off-by: Andrii Sultanov <[email protected]>
@coveralls
Copy link

Coverage Status

coverage: 44.887%. remained the same
when pulling e53ce67 on last-genius:private/asultanov/uri-improvement
into e61e0ac on xapi-project:master.

@robhoes robhoes merged commit 6dd7a48 into xapi-project:master Jul 4, 2024
@last-genius last-genius deleted the private/asultanov/uri-improvement branch July 10, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants