resolve_file does not decode percent encoding

[ewanmellor]

Cohttp_lwt_unix.Server.resolve_file does not decode the percent encoding in the given URI's path, meaning that it returns an incorrect filename for anything with an encoded character. In the code below, Uri.path (from mirage/ocaml-uri) returns the encoded form of the path.

# cohttp-lwt-unix/src/server.ml:

let resolve_file ~docroot ~uri =
  (* This normalises the Uri and strips out .. characters *)
  let frag = Uri.path (Uri.resolve "" blank_uri uri) in
  Filename.concat docroot frag

The fix is easy, but investigating this I found that there are two further implementations of essentially the same function:

# ./cohttp-lwt/src/server.ml:

 let resolve_local_file ~docroot ~uri =
    let path = Uri.(pct_decode (path (resolve "http" (of_string "/") uri))) in
    let rel_path = String.sub path 1 (String.length path - 1) in
    Filename.concat docroot rel_path

# ./cohttp-async/src/server.ml:

let resolve_local_file ~docroot ~uri =
  (* This normalises the Uri and strips out .. characters *)
  Uri.(pct_decode (path (resolve "" (of_string "/") uri)))
  |> Caml.Filename.concat docroot

(Note that these two implementations at least get the percent-decode correct!)

Why do we have three different implementations of the same function? Is there somewhere sensible that we can put a canonical version?

[mseri]

Looks like it has been like that forever, and definitely does not strip the .. as advertised:

# Cohttp_lwt_unix.Server.resolve_file ~docroot:"" ~uri:(Uri.of_string "../../list.dat");;
- : string = "../../list.dat"

# Cohttp_lwt_unix.Server.resolve_local_file ~docroot:"" ~uri:(Uri.of_string "../../list.dat")
;;
- : string = "list.dat"