From 76c2995dbe48d68d9c683beff749ea3b4ec28140 Mon Sep 17 00:00:00 2001 From: Dunya Kokoschka Date: Thu, 29 Apr 2021 11:55:56 +0100 Subject: [PATCH] Ensure content-length header is sent for HTTP/1.1 and HTTP/2 This avoids using chunked transfer encoding for HTTP/1.1. This also ensures the content-length headers is set for HTTP/2. --- lib/mojito/conn.ex | 105 ++++++++++++++++++++++------- test/mojito_test.exs | 22 ++++++ test/support/mojito_test_server.ex | 4 ++ 3 files changed, 107 insertions(+), 24 deletions(-) diff --git a/lib/mojito/conn.ex b/lib/mojito/conn.ex index efe46b2..59c02e9 100644 --- a/lib/mojito/conn.ex +++ b/lib/mojito/conn.ex @@ -63,13 +63,51 @@ defmodule Mojito.Conn do response = %Mojito.Response{body: [], size: max_body_size} with {:ok, relative_url, auth_headers} <- - Utils.get_relative_url_and_auth_headers(request.url), - {:ok, mint_conn, request_ref} <- + Utils.get_relative_url_and_auth_headers(request.url) do + all_headers = auth_headers ++ request.headers + + case conn.conn do + %Mint.HTTP1{} -> + simple_request(conn, relative_url, all_headers, request, response) + + %Mint.HTTP2{} -> + chunk_size = calculate_initial_chunk_size(conn.conn) + body_size = byte_size(request.body || "") + + # we try and avoid the chunking code because it creates an extra empty + # chunk. but otherwise it should have the same behaviour + if body_size <= chunk_size do + simple_request(conn, relative_url, all_headers, request, response) + else + streaming_request( + conn, + relative_url, + add_default_content_length_header(all_headers, request.body), + request, + response + ) + end + end + end + end + + defp add_default_content_length_header(headers, nil) do + headers + end + + defp add_default_content_length_header(headers, body) do + Mint.Core.Util.put_new_header_lazy(headers, "content-length", fn -> + body |> IO.iodata_length() |> Integer.to_string() + end) + end + + defp streaming_request(conn, relative_url, all_headers, request, response) do + with {:ok, mint_conn, request_ref} <- Mint.HTTP.request( conn.conn, method_to_string(request.method), relative_url, - auth_headers ++ request.headers, + all_headers, :stream ), {:ok, mint_conn, response} <- @@ -78,6 +116,44 @@ defmodule Mojito.Conn do end end + defp simple_request(conn, relative_url, all_headers, request, response) do + with {:ok, mint_conn, request_ref} <- + Mint.HTTP.request( + conn.conn, + method_to_string(request.method), + relative_url, + all_headers, + request.body + ) do + {:ok, %{conn | conn: mint_conn}, request_ref, response} + end + end + + defp calculate_initial_chunk_size(mint_conn) do + min( + mint_conn.server_settings.initial_window_size, + Mint.HTTP2.get_window_size(mint_conn, :connection) + ) + end + + defp calculate_chunk_size(mint_conn, request_ref) do + min( + Mint.HTTP2.get_window_size(mint_conn, {:request, request_ref}), + Mint.HTTP2.get_window_size(mint_conn, :connection) + ) + end + + defp binary_split_at(bin, size) do + # don't use String.split_at. split_at is for unicode code points + + if byte_size(bin) <= size do + {bin, ""} + else + <> = bin + {part, rest} + end + end + defp stream_request_body(mint_conn, request_ref, response, nil) do stream_request_body(mint_conn, request_ref, response, "") end @@ -89,33 +165,14 @@ defmodule Mojito.Conn do end end - defp stream_request_body( - %Mint.HTTP1{} = mint_conn, - request_ref, - response, - body - ) do - {chunk, rest} = String.split_at(body, 65535) - - with {:ok, mint_conn} <- - Mint.HTTP.stream_request_body(mint_conn, request_ref, chunk) do - stream_request_body(mint_conn, request_ref, response, rest) - end - end - defp stream_request_body( %Mint.HTTP2{} = mint_conn, request_ref, response, body ) do - chunk_size = - min( - Mint.HTTP2.get_window_size(mint_conn, {:request, request_ref}), - Mint.HTTP2.get_window_size(mint_conn, :connection) - ) - - {chunk, rest} = String.split_at(body, chunk_size) + chunk_size = calculate_chunk_size(mint_conn, request_ref) + {chunk, rest} = binary_split_at(body, chunk_size) with {:ok, mint_conn} <- Mint.HTTP.stream_request_body(mint_conn, request_ref, chunk) do diff --git a/test/mojito_test.exs b/test/mojito_test.exs index 3640f8e..0739a19 100644 --- a/test/mojito_test.exs +++ b/test/mojito_test.exs @@ -216,6 +216,28 @@ defmodule MojitoTest do assert("12" == Headers.get(response.headers, "content-length")) end + it "sends content-length on http/1.1 requests" do + assert({:ok, response} = post("/headers", "body", protocols: [:http1])) + headers = Jason.decode!(response.body) + + assert Map.get(headers, "content-length") == "6" + end + + it "sends content-length on http2 requests" do + assert({:ok, response} = post("/headers", "body", protocols: [:http2])) + headers = Jason.decode!(response.body) + + assert Map.get(headers, "content-length") == "6" + end + + it "sends content-length on large http2 requests" do + big = String.duplicate("x", 5_000_000) + assert({:ok, response} = post("/headers", big, protocols: [:http2])) + headers = Jason.decode!(response.body) + + assert Map.get(headers, "content-length") == "5000002" + end + it "handles timeouts" do assert({:ok, _} = get("/", timeout: 100)) assert({:error, %Error{reason: :timeout}} = get("/wait1", timeout: 100)) diff --git a/test/support/mojito_test_server.ex b/test/support/mojito_test_server.ex index 4986970..a51d2fc 100644 --- a/test/support/mojito_test_server.ex +++ b/test/support/mojito_test_server.ex @@ -59,6 +59,10 @@ defmodule Mojito.TestServer.PlugRouter do send_resp(conn, 200, Jason.encode!(%{name: name})) end + post "/headers" do + send_resp(conn, 200, Jason.encode!(Map.new(conn.req_headers))) + end + patch "/patch" do name = conn.body_params["name"] || "Bob" send_resp(conn, 200, Jason.encode!(%{name: name}))