diff --git a/CHANGES.md b/CHANGES.md index e7f88aa..68c1b35 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,22 +1,6 @@ Unreleased -------------- -- httpaf-lwt-unix: replace the `dune` file (previously written in OCaml) with a - `(select)` form to avoid depending on `ocamlfind` - ([#18](https://github.com/anmonteiro/httpaf/pull/18)) -- httpaf-mirage: depend on `mirage-conduit` instead of `conduit-mirage`, - effectively placing a lower bound of OCaml 4.07 on httpaf-mirage - ([#16](https://github.com/anmonteiro/httpaf/pull/16)) -- httpaf-lwt, httpaf-lwt-unix, httpaf-mirage: deduplicate interface code via a - common `Httpaf_lwt_intf` interface - ([#13](https://github.com/anmonteiro/httpaf/pull/13)) -- httpaf, httpaf-lwt, httpaf-lwt-unix, httpaf-async, httpaf-mirage: add support - for persistent connections and pipelining in the client implementations - ([#5](https://github.com/anmonteiro/httpaf/pull/5)) -- httpaf, httpaf-lwt, httpaf-lwt-unix, httpaf-async, httpaf-mirage: add support - for switching protocols, e.g. to a WebSocket connection, via a new function - `Reqd.respond_with_upgrade` - ([#8](https://github.com/anmonteiro/httpaf/pull/8)) - httpaf-lwt-unix: add support for HTTPS in the Lwt runtime, either via OpenSSL or `ocaml-tls` ([#2](https://github.com/anmonteiro/httpaf/pull/2)) - httpaf, httpaf-lwt, httpaf-lwt-unix, httpaf-mirage: Add a Mirage adapter @@ -24,11 +8,27 @@ Unreleased - Add an `esy.json` file ([#6](https://github.com/anmonteiro/httpaf/pull/6)) - httpaf: Catch parsing errors and hand them to `error_handler` in `Server_connection` ([#4](https://github.com/anmonteiro/httpaf/pull/4)) +- httpaf, httpaf-lwt, httpaf-lwt-unix, httpaf-async, httpaf-mirage: add support + for persistent connections and pipelining in the client implementations + ([#5](https://github.com/anmonteiro/httpaf/pull/5)) +- httpaf, httpaf-lwt, httpaf-lwt-unix, httpaf-async, httpaf-mirage: add support + for switching protocols, e.g. to a WebSocket connection, via a new function + `Reqd.respond_with_upgrade` + ([#8](https://github.com/anmonteiro/httpaf/pull/8)) +- httpaf-lwt, httpaf-lwt-unix, httpaf-mirage: deduplicate interface code via a + common `Httpaf_lwt_intf` interface + ([#13](https://github.com/anmonteiro/httpaf/pull/13)) +- httpaf-mirage: depend on `mirage-conduit` instead of `conduit-mirage`, + effectively placing a lower bound of OCaml 4.07 on httpaf-mirage + ([#16](https://github.com/anmonteiro/httpaf/pull/16)) +- httpaf-lwt-unix: replace the `dune` file (previously written in OCaml) with a + `(select)` form to avoid depending on `ocamlfind` + ([#18](https://github.com/anmonteiro/httpaf/pull/18)) +- httpaf: Shutdown the writer after closing a non chunk-encoded request body on + the client ([#23](https://github.com/anmonteiro/httpaf/pull/23)) - httpaf-mirage: Adapt to Mirage 3.7 interfaces. `httpaf_mirage` now requires `conduit-mirage` >= 2.0.2 and `mirage-flow` >= 2.0.0 ([#24](https://github.com/anmonteiro/httpaf/pull/24)) -- httpaf: Shutdown the writer after closing a non chunk-encoded request body on - the client ([#23](https://github.com/anmonteiro/httpaf/pull/23)) - httpaf, httpaf-lwt, httpaf-lwt-unix, httpaf-async, httpaf-mirage: after switching protocols, close the connection / file descriptors when the upgrade handler's returned promise resolves @@ -48,6 +48,11 @@ Unreleased advertised in the interface file. They are now served to the respective handlers in the original transmission order ([#32](https://github.com/anmonteiro/httpaf/pull/32)) +- httpaf: Fix persistent connections getting stuck (reader never waking up) + when using `~flush_headers_immediately:true` in combination with an empty + response body ([#34](https://github.com/anmonteiro/httpaf/pull/34)). This is + a fix for an issue opened in the upstream repo: + [inhabitedtype/httpaf#162](https://github.com/inhabitedtype/httpaf/issues/162) httpaf (upstream) 0.6.5 -------------- diff --git a/lib/body.ml b/lib/body.ml index df911f2..bcb0848 100644 --- a/lib/body.ml +++ b/lib/body.ml @@ -157,6 +157,20 @@ let when_ready_to_write t callback = let transfer_to_writer_with_encoding t ~encoding writer = let faraday = t.faraday in + begin match t.write_final_if_chunked, encoding with + | true, (`Fixed _ | `Close_delimited) -> + (* Play nicely with [has_pending_output] in the case of a fixed or + close-delimited encoding. + + Immediately set `t.write_final_if_chunked` to `false` because we may + not have another opportunity to do so before advancing the request + queue. *) + t.write_final_if_chunked <- false; + | false, (`Fixed _ | `Close_delimited) + | _, `Chunked -> + (* Handled explicitly later when closing the writer. *) + () + end; begin match Faraday.operation faraday with | `Yield -> () | `Close -> @@ -178,9 +192,6 @@ let transfer_to_writer_with_encoding t ~encoding writer = buffered := !buffered + lengthv; begin match encoding with | `Fixed _ | `Close_delimited -> - (* Play nicely with [has_pending_output] in the case of a fixed or - close-delimited encoding. *) - t.write_final_if_chunked <- false; Serialize.Writer.schedule_fixed writer iovecs | `Chunked -> Serialize.Writer.schedule_chunk writer iovecs diff --git a/lib_test/test_httpaf.ml b/lib_test/test_httpaf.ml index 63a8982..53f8d5d 100644 --- a/lib_test/test_httpaf.ml +++ b/lib_test/test_httpaf.ml @@ -836,6 +836,46 @@ Accept-Language: en-US,en;q=0.5\r\n\r\n"; writer_closed t; ;; + let test_immediate_flush_empty_body () = + let reader_woken_up = ref false in + let response = Response.create `OK in + let request_handler reqd = + let resp_body = Reqd.respond_with_streaming + ~flush_headers_immediately:true reqd response + in + Body.close_writer resp_body; + in + let t = create ~error_handler request_handler in + reader_ready t; + yield_writer t (fun () -> + write_response t ~body:"" response); + read_request t (Request.create `GET "/"); + yield_reader t (fun () -> reader_woken_up := true); + writer_yielded t; + Alcotest.(check bool) "Reader woken up" + true !reader_woken_up; + ;; + + let test_empty_body_no_immediate_flush () = + let reader_woken_up = ref false in + let response = Response.create `OK in + let request_handler reqd = + let resp_body = Reqd.respond_with_streaming + ~flush_headers_immediately:false reqd response + in + Body.close_writer resp_body; + in + let t = create ~error_handler request_handler in + reader_ready t; + writer_yielded t; + read_request t (Request.create `GET "/"); + yield_reader t (fun () -> reader_woken_up := true); + write_response t ~body:"" response; + writer_yielded t; + Alcotest.(check bool) "Reader woken up" + true !reader_woken_up; + ;; + let tests = [ "initial reader state" , `Quick, test_initial_reader_state ; "shutdown reader closed", `Quick, test_reader_is_closed_after_eof @@ -859,6 +899,8 @@ Accept-Language: en-US,en;q=0.5\r\n\r\n"; ; "respond with upgrade", `Quick, test_respond_with_upgrade ; "writer unexpected eof", `Quick, test_unexpected_eof ; "input shrunk", `Quick, test_input_shrunk + ; "`flush_headers_immediately` with empty body", `Quick, test_immediate_flush_empty_body + ; "empty body with no immediate flush", `Quick, test_empty_body_no_immediate_flush ] end