From c217c2e523d01e86765f0138d4604ea13209eae2 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Fri, 9 Apr 2021 20:25:57 -0700 Subject: [PATCH] client: close request body immediately if no payload is expected (#92) --- lib/client_connection.ml | 19 ++++++++++++++----- lib_test/test_client_connection.ml | 30 ++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/lib/client_connection.ml b/lib/client_connection.ml index 76107484..f5b8401f 100644 --- a/lib/client_connection.ml +++ b/lib/client_connection.ml @@ -78,14 +78,23 @@ let[@ocaml.warning "-16"] create ?(config=Config.default) = ; request_queue } +let create_request_body t = + Body.create + (Bigstringaf.create t.config.request_body_buffer_size) + (Optional_thunk.some (fun () -> wakeup_writer t)) + let request t request ~error_handler ~response_handler = let request_body = - Body.create - (Bigstringaf.create t.config.request_body_buffer_size) - (Optional_thunk.some (fun () -> wakeup_writer t)) + match Request.body_length request with + | `Fixed 0L -> Body.empty + | `Chunked -> create_request_body t + | `Fixed _ -> + let body = create_request_body t in + Body.set_non_chunked body; + body + | `Error _ -> + assert false in - if not (Request.body_length request = `Chunked) then - Body.set_non_chunked request_body; let respd = Respd.create error_handler request request_body t.writer response_handler in let handle_now = Queue.is_empty t.request_queue in diff --git a/lib_test/test_client_connection.ml b/lib_test/test_client_connection.ml index c86effaf..50dc8f5b 100644 --- a/lib_test/test_client_connection.ml +++ b/lib_test/test_client_connection.ml @@ -551,7 +551,7 @@ let test_empty_fixed_body () = Body.close_reader response_body in let t = create ?config:None in - let body = + let (_body: [`write] Body.t) = request t request' @@ -559,14 +559,12 @@ let test_empty_fixed_body () = ~error_handler:no_error_handler in write_request t request'; - writer_yielded t; - Body.close_writer body; + writer_closed t; reader_ready t; read_response t (Response.create ~headers:(Headers.of_list ["Connection", "close"]) `OK); let c = read_eof t Bigstringaf.empty ~off:0 ~len:0 in Alcotest.(check int) "read_eof with no input returns 0" 0 c; reader_closed t; - writer_closed t; ;; let test_fixed_body () = @@ -1396,6 +1394,29 @@ let test_304_not_modified () = Alcotest.(check bool) "error handler not called" false !error_handler_called; ;; +let test_empty_content_length_body_closed () = + let response = Response.create `OK in + let request' = Request.create `GET "/" in + let t = create ?config:None in + let body = + request + t + request' + ~response_handler:(default_response_handler response) + ~error_handler:no_error_handler + in + Alcotest.(check bool) "request body length is 0" + true + ((Request.body_length request') = (`Fixed 0L)); + Alcotest.(check bool) "Request body starts out closed" true (Body.is_closed body); + write_request t request'; + read_response t response; + let c = read_eof t Bigstringaf.empty ~off:0 ~len:0 in + Alcotest.(check int) "read_eof with no input returns 0" 0 c; + connection_is_shutdown t; +;; + + let tests = [ "commit parse after every header line", `Quick, test_commit_parse_after_every_header ; "GET" , `Quick, test_get @@ -1432,4 +1453,5 @@ let tests = ; "multiple responses in single read", `Quick, test_multiple_responses_in_single_read ; "test chunked error", `Quick, test_chunked_error ; "304 Not Modified with Content-Length", `Quick, test_304_not_modified + ; "body for a request with no payload starts out closed", `Quick, test_empty_content_length_body_closed ]