Skip to content
This repository has been archived by the owner on May 21, 2022. It is now read-only.

Ensure content-length header is sent for HTTP/1.1 and HTTP/2 #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 81 additions & 24 deletions lib/mojito/conn.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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} <-
Expand All @@ -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
<<part::binary-size(size), rest::binary()>> = 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
Expand All @@ -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
Expand Down
22 changes: 22 additions & 0 deletions test/mojito_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 4 additions & 0 deletions test/support/mojito_test_server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}))
Expand Down