diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml index 66f0eab267..3a7b171442 100644 --- a/.github/workflows/nix.yml +++ b/.github/workflows/nix.yml @@ -5,6 +5,8 @@ on: jobs: tests: runs-on: ubuntu-latest + env: + NIXPKGS_ALLOW_INSECURE: 1 steps: - name: Checkout code uses: actions/checkout@v2 diff --git a/.ocamlformat b/.ocamlformat index 412e7fc48a..0b4769bef5 100644 --- a/.ocamlformat +++ b/.ocamlformat @@ -1,4 +1,4 @@ -version = 0.24.1 +version = 0.25.1 profile=conventional break-infix=fit-or-vertical parse-docstrings=true diff --git a/CHANGES.md b/CHANGES.md index fb17903697..c535dd4101 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,7 @@ -## v6.0.0~alpha2 (2023-06-20) - +## v6.0.0~alpha2 (2023-07-1) +- http.header: introduce "iter_ord" to guarantee iteration following the order of the entries in the headers (mseri #986) +- http.header: fix "move_to_fist" and "first" ro follow Header's semantics (mseri #986) +- cohttp: ensure "host" is the first header (mseri #986) - do not omit mandatory null Content-Length headers (mefyl #985) - cohttp-async, cohttp-curl-async: compatibility with core/async v0.16.0 (mseri, dkalinichenko-js #976) - cohttp-lwt server: call conn_closed before drainig the body of response on error (pirbo #982) diff --git a/cohttp-async/examples/s3_cp.ml b/cohttp-async/examples/s3_cp.ml index 0c95d7bd64..f0e8687fbc 100644 --- a/cohttp-async/examples/s3_cp.ml +++ b/cohttp-async/examples/s3_cp.ml @@ -43,6 +43,8 @@ open Base open Core open Async +module Time = Time_float + (* open Cohttp *) module Client = Cohttp_async.Client module Body = Cohttp_async.Body diff --git a/cohttp-async/test/test_async_integration.ml b/cohttp-async/test/test_async_integration.ml index d709759f0e..91bec398f4 100644 --- a/cohttp-async/test/test_async_integration.ml +++ b/cohttp-async/test/test_async_integration.ml @@ -113,7 +113,8 @@ let ts = ("Pipe with empty strings", Pipe.of_list [ ""; ""; "" ], true); ] in - Deferred.List.iter ~how:`Sequential tests ~f:(fun (msg, pipe, expected) -> + Deferred.List.iter ~how:`Sequential tests + ~f:(fun (msg, pipe, expected) -> is_empty (`Pipe pipe) >>| fun real -> assert_equal ~msg expected real) >>= fun () -> diff --git a/cohttp-eio/src/rwer.ml b/cohttp-eio/src/rwer.ml index ac3d56ddc5..bb7c2610bb 100644 --- a/cohttp-eio/src/rwer.ml +++ b/cohttp-eio/src/rwer.ml @@ -53,7 +53,7 @@ let http_headers r = let write_headers writer headers = let headers = Http.Header.clean_dup headers in - Http.Header.iter + Http.Header.iter_ord (fun k v -> Buf_write.string writer k; Buf_write.string writer ": "; diff --git a/cohttp-eio/tests/client.md b/cohttp-eio/tests/client.md index 98fcd16fbe..9520785302 100644 --- a/cohttp-eio/tests/client.md +++ b/cohttp-eio/tests/client.md @@ -45,10 +45,10 @@ GET method request: |> print_string);; +socket: wrote "GET / HTTP/1.1\r\n" + "Host: localhost\r\n" -+ "Connection: TE\r\n" -+ "TE: trailers\r\n" -+ "User-Agent: cohttp-eio\r\n" + "Accept: application/json\r\n" ++ "User-Agent: cohttp-eio\r\n" ++ "TE: trailers\r\n" ++ "Connection: TE\r\n" + "\r\n" +socket: read "HTTP/1.1 200 OK\r\n" +socket: read "content-length: 4\r\n" @@ -83,11 +83,11 @@ POST request: |> print_string);; +socket: wrote "POST /post HTTP/1.1\r\n" + "Host: localhost\r\n" -+ "Connection: TE\r\n" -+ "TE: trailers\r\n" -+ "User-Agent: cohttp-eio\r\n" -+ "Content-Length: 12\r\n" + "Accept: application/json\r\n" ++ "Content-Length: 12\r\n" ++ "User-Agent: cohttp-eio\r\n" ++ "TE: trailers\r\n" ++ "Connection: TE\r\n" + "\r\n" + "hello world!" +socket: read "HTTP/1.1 200 OK\r\n" @@ -159,12 +159,12 @@ Chunk request: |> print_string);; +socket: wrote "POST /handle_chunk HTTP/1.1\r\n" + "Host: localhost\r\n" -+ "Connection: TE\r\n" -+ "TE: trailers\r\n" -+ "User-Agent: cohttp-eio\r\n" -+ "Trailer: Expires, Header1\r\n" -+ "Content-Type: text/plain\r\n" + "Transfer-Encoding: chunked\r\n" ++ "Content-Type: text/plain\r\n" ++ "Trailer: Expires, Header1\r\n" ++ "User-Agent: cohttp-eio\r\n" ++ "TE: trailers\r\n" ++ "Connection: TE\r\n" + "\r\n" + "7;ext1=ext1_v;ext2=ext2_v;ext3\r\n" + "Mozilla\r\n" @@ -173,9 +173,9 @@ Chunk request: + "7\r\n" + "Network\r\n" + "0\r\n" -+ "Header2: Header2 value text\r\n" -+ "Header1: Header1 value text\r\n" + "Expires: Wed, 21 Oct 2015 07:28:00 GMT\r\n" ++ "Header1: Header1 value text\r\n" ++ "Header2: Header2 value text\r\n" + "\r\n" +socket: read "HTTP/1.1 200 OK\r\n" +socket: read "content-length:0\r\n" @@ -209,9 +209,9 @@ Chunk request: );; +socket: wrote "GET /get_chunk HTTP/1.1\r\n" + "Host: localhost\r\n" -+ "Connection: TE\r\n" -+ "TE: trailers\r\n" + "User-Agent: cohttp-eio\r\n" ++ "TE: trailers\r\n" ++ "Connection: TE\r\n" + "\r\n" +socket: read "HTTP/1.1 200 OK\r\n" +socket: read "Trailer: Expires, Header1\r\n" diff --git a/cohttp-eio/tests/server.md b/cohttp-eio/tests/server.md index 781b460065..02ad9cd913 100644 --- a/cohttp-eio/tests/server.md +++ b/cohttp-eio/tests/server.md @@ -137,9 +137,9 @@ Asking for the root: +socket: read "GET / HTTP/1.1\r\n" + "\r\n" +socket: wrote "HTTP/1.1 200 OK\r\n" -+ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" -+ "content-length: 4\r\n" + "content-type: text/plain; charset=UTF-8\r\n" ++ "content-length: 4\r\n" ++ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" + "\r\n" + "root" - : unit = () @@ -156,8 +156,8 @@ A missing page: +socket: read "GET /missing HTTP/1.1\r\n" + "\r\n" +socket: wrote "HTTP/1.1 404 Not Found\r\n" -+ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" + "Content-Length: 0\r\n" ++ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" + "\r\n" - : unit = () ``` @@ -173,8 +173,8 @@ Streaming a response: +socket: read "GET /stream HTTP/1.1\r\n" + "\r\n" +socket: wrote "HTTP/1.1 200 OK\r\n" -+ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" + "transfer-encoding: chunked\r\n" ++ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" + "\r\n" + "5\r\n" + "Hello\r\n" @@ -201,9 +201,9 @@ Handle POST request: + "\r\n" +socket: read "hello world!" +socket: wrote "HTTP/1.1 200 OK\r\n" -+ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" -+ "content-length: 100\r\n" + "content-type: text/plain; charset=UTF-8\r\n" ++ "content-length: 100\r\n" ++ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" + "\r\n" + "meth: POST\n" + "resource: /post\n" @@ -227,10 +227,10 @@ HTTP chunk-stream response with chunk extensions and trailers: +socket: read "TE:trailers\r\n" + "\r\n" +socket: wrote "HTTP/1.1 200 OK\r\n" -+ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" -+ "Trailer: Expires, Header1\r\n" -+ "Content-Type: text/plain\r\n" + "Transfer-Encoding: chunked\r\n" ++ "Content-Type: text/plain\r\n" ++ "Trailer: Expires, Header1\r\n" ++ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" + "\r\n" + "7;ext1=ext1_v;ext2=ext2_v;ext3\r\n" + "Mozilla\r\n" @@ -239,9 +239,9 @@ HTTP chunk-stream response with chunk extensions and trailers: + "7\r\n" + "Network\r\n" + "0\r\n" -+ "Header2: Header2 value text\r\n" -+ "Header1: Header1 value text\r\n" + "Expires: Wed, 21 Oct 2015 07:28:00 GMT\r\n" ++ "Header1: Header1 value text\r\n" ++ "Header2: Header2 value text\r\n" + "\r\n" - : unit = () ``` @@ -259,10 +259,10 @@ a HTTP client agent has support for HTTP chunk trailer headers: +socket: read "GET /get_chunks HTTP/1.1\r\n" + "\r\n" +socket: wrote "HTTP/1.1 200 OK\r\n" -+ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" -+ "Trailer: Expires, Header1\r\n" -+ "Content-Type: text/plain\r\n" + "Transfer-Encoding: chunked\r\n" ++ "Content-Type: text/plain\r\n" ++ "Trailer: Expires, Header1\r\n" ++ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" + "\r\n" + "7;ext1=ext1_v;ext2=ext2_v;ext3\r\n" + "Mozilla\r\n" @@ -313,9 +313,9 @@ Server should handle chunk requests from clients: +socket: read "Header2: Header2 value text\r\n" + "\r\n" +socket: wrote "HTTP/1.1 200 OK\r\n" -+ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" -+ "content-length: 354\r\n" + "content-type: text/plain; charset=UTF-8\r\n" ++ "content-length: 354\r\n" ++ "Date: Mon, 24 Oct 2022 16:12:15 GMT\r\n" + "\r\n" + "meth: POST\n" + "resource: /handle_chunk\n" diff --git a/cohttp-lwt-unix/src/debug.ml b/cohttp-lwt-unix/src/debug.ml index 2d550017ab..fd2152a51b 100644 --- a/cohttp-lwt-unix/src/debug.ml +++ b/cohttp-lwt-unix/src/debug.ml @@ -67,10 +67,10 @@ let default_reporter = reporter Lwt_unix.stderr Fmt.stderr let set_logger = lazy (if - (* If no reporter has been set by the application, set default one - that prints to stderr *) - Logs.reporter () == Logs.nop_reporter - then Logs.set_reporter default_reporter) + (* If no reporter has been set by the application, set default one + that prints to stderr *) + Logs.reporter () == Logs.nop_reporter + then Logs.set_reporter default_reporter) let activate_debug () = if not !_debug_active then ( diff --git a/cohttp-lwt-unix/test/test_parser.ml b/cohttp-lwt-unix/test/test_parser.ml index 05af2d559b..ecd7ec6f6e 100644 --- a/cohttp-lwt-unix/test/test_parser.ml +++ b/cohttp-lwt-unix/test/test_parser.ml @@ -270,7 +270,7 @@ let make_simple_req () = let open Cohttp in let open Cohttp_lwt_unix in let expected = - "POST /foo/bar HTTP/1.1\r\nFoo: bar\r\nhost: localhost\r\nuser-agent: " + "POST /foo/bar HTTP/1.1\r\nhost: localhost\r\nFoo: bar\r\nuser-agent: " ^ user_agent ^ "\r\ntransfer-encoding: chunked\r\n\r\n6\r\nfoobar\r\n0\r\n\r\n" in @@ -285,7 +285,7 @@ let mutate_simple_req () = let open Cohttp in let open Cohttp_lwt_unix in let expected = - "POST /foo/bar HTTP/1.1\r\nfoo: bar\r\nhost: localhost\r\nuser-agent: " + "POST /foo/bar HTTP/1.1\r\nhost: localhost\r\nfoo: bar\r\nuser-agent: " ^ user_agent ^ "\r\ntransfer-encoding: chunked\r\n\r\n6\r\nfoobar\r\n0\r\n\r\n" in diff --git a/cohttp-lwt/src/connection.ml b/cohttp-lwt/src/connection.ml index 02658a3160..13e7525da6 100644 --- a/cohttp-lwt/src/connection.ml +++ b/cohttp-lwt/src/connection.ml @@ -117,34 +117,34 @@ module Make (Net : S.Net) : S.Connection with module Net = Net = struct (* A response header to a HEAD request is indistinguishable from a * response header to a GET request. Therefore look at the method. *) (if - match Response.has_body res with - | _ when meth = `HEAD -> false - | `No -> false - | `Yes | `Unknown -> true - then ( - let stream = - Body.create_stream Response.read_body_chunk - (Response.make_body_reader res ic) - in - (* finalise could run in a thread different from the lwt main thread. - * You may therefore not call into Lwt from a finaliser. *) - let closed = ref false in - Gc.finalise_last - (fun () -> - if not !closed then - Log.warn (fun m -> - m - "Body not consumed, leaking stream! Refer to \ - https://github.com/mirage/ocaml-cohttp/issues/730 \ - for additional details")) - stream; - Lwt.wakeup_later res_r (res, Body.of_stream stream); - Lwt_stream.closed stream >>= fun () -> - closed := true; - Lwt.return_unit) - else ( - Lwt.wakeup_later res_r (res, `Empty); - Lwt.return_unit)) + match Response.has_body res with + | _ when meth = `HEAD -> false + | `No -> false + | `Yes | `Unknown -> true + then ( + let stream = + Body.create_stream Response.read_body_chunk + (Response.make_body_reader res ic) + in + (* finalise could run in a thread different from the lwt main thread. + * You may therefore not call into Lwt from a finaliser. *) + let closed = ref false in + Gc.finalise_last + (fun () -> + if not !closed then + Log.warn (fun m -> + m + "Body not consumed, leaking stream! Refer to \ + https://github.com/mirage/ocaml-cohttp/issues/730 \ + for additional details")) + stream; + Lwt.wakeup_later res_r (res, Body.of_stream stream); + Lwt_stream.closed stream >>= fun () -> + closed := true; + Lwt.return_unit) + else ( + Lwt.wakeup_later res_r (res, `Empty); + Lwt.return_unit)) >>= fun () -> Queue.take connection.in_flight |> ignore; Lwt_condition.broadcast connection.condition (); diff --git a/cohttp/src/header.mli b/cohttp/src/header.mli index 7cd1983aa4..37bf7d6d0a 100644 --- a/cohttp/src/header.mli +++ b/cohttp/src/header.mli @@ -187,9 +187,9 @@ val clean_dup : t -> t host: anhost.com, anotherhost.com v} - Finally, following {{:https://tools.ietf.org/html/rfc7230#section-3.2.2} - RFC7230§3.2.2}, the header [Set-cookie] is treated as an exception and - ignored by [clean_dup]. *) + Finally, following + {{:https://tools.ietf.org/html/rfc7230#section-3.2.2} RFC7230§3.2.2}, the + header [Set-cookie] is treated as an exception and ignored by [clean_dup]. *) val get_content_range : t -> Int64.t option val get_media_type : t -> string option diff --git a/cohttp/src/request.ml b/cohttp/src/request.ml index f60fd00fbf..96a251932c 100644 --- a/cohttp/src/request.ml +++ b/cohttp/src/request.ml @@ -61,6 +61,7 @@ let make ?(meth = `GET) ?(version = `HTTP_1_1) ?(encoding = Transfer.Unknown) ^ match Uri.port uri with Some p -> ":" ^ string_of_int p | None -> "")) in + let headers = Header.Private.move_to_front headers "host" in let headers = Header.add_unless_exists headers "user-agent" Header.user_agent in @@ -203,6 +204,7 @@ module Make (IO : S.IO) = struct Header.add_transfer_encoding headers req.encoding else headers in + let headers = Header.Private.move_to_front headers "host" in IO.write oc fst_line >>= fun _ -> Header_IO.write headers oc let make_body_writer ?flush req oc = diff --git a/cohttp/src/transfer_io.ml b/cohttp/src/transfer_io.ml index 5d4b90618a..adce81a1d7 100644 --- a/cohttp/src/transfer_io.ml +++ b/cohttp/src/transfer_io.ml @@ -43,8 +43,8 @@ module Make (IO : S.IO) = struct read_chunk ic !remaining >>= fun chunk -> remaining := remaining_length chunk !remaining; (if !remaining = 0L (* End_of_chunk *) then read_line ic - (* Junk the CRLF at end of chunk *) - else return None) + (* Junk the CRLF at end of chunk *) + else return None) >>= fun _ -> return chunk in if !remaining = 0L then diff --git a/cohttp/test/test_request.ml b/cohttp/test/test_request.ml index 28c734487e..bc999c22c8 100644 --- a/cohttp/test/test_request.ml +++ b/cohttp/test/test_request.ml @@ -308,8 +308,12 @@ module Request = Request.Private.Make (Test_io) let null_content_length_header () = let output = Buffer.create 1024 in let () = + (* The user-agent in releases contentsontains the version, we need to strip + it for the test *) + let headers = Cohttp.Header.of_list [ ("user-agent", "ocaml-cohttp") ] in let r = - Cohttp.Request.make_for_client ~chunked:false ~body_length:0L `PUT + Cohttp.Request.make_for_client ~headers ~chunked:false ~body_length:0L + `PUT (Uri.of_string "http://someuri.com") in Request.write_header r output @@ -318,7 +322,7 @@ let null_content_length_header () = "null content-length header are sent" "PUT / HTTP/1.1\r\n\ host: someuri.com\r\n\ - user-agent: ocaml-cohttp/\r\n\ + user-agent: ocaml-cohttp\r\n\ content-length: 0\r\n\ \r\n" (Buffer.to_string output) diff --git a/http/src/http.ml b/http/src/http.ml index 6301943fd6..6968ed1922 100644 --- a/http/src/http.ml +++ b/http/src/http.ml @@ -162,14 +162,20 @@ module Header = struct add_multi h k xs let move_to_front t hdr_name = - match t with - | (k, _) :: _ when caseless_equal k hdr_name -> t - | _ -> ( - match get t hdr_name with - | Some v -> - let headers = remove t hdr_name in - add headers hdr_name v - | None -> t) + (* Headers are manipulated in reverse order for convenience, so we + need to reverse them, prepend what we need, and reverse again *) + let t = List.rev t in + let t = + match t with + | (k, _) :: _ when caseless_equal k hdr_name -> t + | _ -> ( + match get t hdr_name with + | Some v -> + let headers = remove t hdr_name in + add headers hdr_name v + | None -> t) + in + List.rev t let map (f : string -> string -> string) (h : t) : t = List.map @@ -178,6 +184,9 @@ module Header = struct (k, vs')) h + let iter_ord (f : string -> string -> unit) (h : t) : unit = + List.iter (fun (k, v) -> f k v) (List.rev h) + let iter (f : string -> string -> unit) (h : t) : unit = List.iter (fun (k, v) -> f k v) h @@ -349,7 +358,7 @@ module Header = struct module Private = struct let caseless_equal = caseless_equal - let first = first + let first l = first (List.rev l) let move_to_front = move_to_front end end diff --git a/http/src/http.mli b/http/src/http.mli index 6c1a3f63e8..45eb019fd1 100644 --- a/http/src/http.mli +++ b/http/src/http.mli @@ -308,7 +308,13 @@ module Header : sig - If [k] was already associated in [h] to a list that is equal to [vs], [h] is returned unchanged. *) + val iter_ord : (string -> string -> unit) -> t -> unit + (** [iter_ord f h] applies [f] to all the headers of [h] following the header order. *) + val iter : (string -> string -> unit) -> t -> unit + (** [iter f h] applies [f] to all the headers of [h] following an unspecified order. + This function is faster than iter_ord. *) + val map : (string -> string -> string) -> t -> t val fold : (string -> string -> 'a -> 'a) -> t -> 'a -> 'a @@ -348,9 +354,9 @@ module Header : sig host: anhost.com, anotherhost.com v} - Finally, following {{:https://tools.ietf.org/html/rfc7230#section-3.2.2} - RFC7230§3.2.2}, the header [Set-cookie] is treated as an exception and - ignored by [clean_dup]. *) + Finally, following + {{:https://tools.ietf.org/html/rfc7230#section-3.2.2} RFC7230§3.2.2}, the + header [Set-cookie] is treated as an exception and ignored by [clean_dup]. *) val get_content_range : t -> Int64.t option val get_connection_close : t -> bool