From 2549e1fd9589ddebb23da09903ad3378e9c453b5 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Thu, 25 Jul 2024 23:06:20 +0200 Subject: [PATCH 01/11] Add Mint.HTTP1.recv_response/3,5 --- lib/mint/http1.ex | 130 ++++++++++++++++++++++++++++++++++ test/mint/http1/conn_test.exs | 30 +++++++- 2 files changed, 159 insertions(+), 1 deletion(-) diff --git a/lib/mint/http1.ex b/lib/mint/http1.ex index 3c543d5c..3447b7d0 100644 --- a/lib/mint/http1.ex +++ b/lib/mint/http1.ex @@ -529,6 +529,136 @@ defmodule Mint.HTTP1 do "Use Mint.HTTP.set_mode/2 to set the connection to passive mode" end + @doc """ + TODO + + ## Examples + + iex> {:ok, conn} = Mint.HTTP1.connect(:https, "httpbin.org", 443, mode: :passive) + iex> {:ok, conn, ref} = Mint.HTTP1.request(conn, "GET", "/status/user-agent", [], nil) + iex> {:ok, _conn, response} = Mint.HTTP1.recv_response(conn, ref, 5000) + iex> response + %{ + status: 201, + headers: [ + {"date", ...}, + ... + ], + body: "{\\n \\"user-agent\\": \\"mint/1.6.2\\"\\n}\\n" + } + """ + def recv_response(conn, ref, timeout) do + recv_response(conn, ref, timeout, %{status: nil, headers: [], body: ""}, fn + conn, {:status, status}, acc -> + {:cont, conn, %{acc | status: status}} + + conn, {:headers, headers}, acc -> + {:cont, conn, update_in(acc.headers, &(&1 ++ headers))} + + conn, {:data, data}, acc -> + {:cont, conn, update_in(acc.body, &(&1 <> data))} + + conn, :done, acc -> + {:cont, conn, acc} + end) + end + + @doc """ + TODO + + ## Examples + + iex> {:ok, conn} = Mint.HTTP1.connect(:https, "httpbin.org", 443, mode: :passive) + iex> {:ok, conn, ref} = Mint.HTTP1.request(conn, "GET", "/user-agent", [], nil) + iex> {:ok, _conn, _acc} = + ...> Mint.HTTP1.recv_response(conn, ref, 5000, nil, fn + ...> conn, entry, acc -> + ...> IO.inspect(entry) + ...> {:cont, conn, acc} + ...> end) + + Outputs: + + {:status, 201} + {:headers, [{"date", ...}, ...]} + {:data, "{\\n \\"user-agent\\": \\"mint/1.6.2\\"\\n}\\n"} + :done + """ + def recv_response(conn, ref, timeout, acc, fun) do + recv_response([], acc, fun, conn, ref, timeout) + end + + defp recv_response([{:status, ref, status} | rest], acc, fun, conn, ref, timeout) do + case fun.(conn, {:status, status}, acc) do + {:cont, conn, acc} -> + recv_response(rest, acc, fun, conn, ref, timeout) + + {:halt, conn, acc} -> + {:ok, conn, acc} + + other -> + raise ArgumentError, + "expected {:cont, conn, acc} or {:halt, conn, acc}, got: #{inspect(other)}" + end + end + + defp recv_response([{:headers, ref, headers} | rest], acc, fun, conn, ref, timeout) do + case fun.(conn, {:headers, headers}, acc) do + {:cont, conn, acc} -> + recv_response(rest, acc, fun, conn, ref, timeout) + + {:halt, conn, acc} -> + {:ok, conn, acc} + + other -> + raise ArgumentError, + "expected {:cont, conn, acc} or {:halt, conn, acc}, got: #{inspect(other)}" + end + end + + defp recv_response([{:data, ref, data} | rest], acc, fun, conn, ref, timeout) do + case fun.(conn, {:data, data}, acc) do + {:cont, conn, acc} -> + recv_response(rest, acc, fun, conn, ref, timeout) + + {:halt, conn, acc} -> + {:ok, conn, acc} + + other -> + raise ArgumentError, + "expected {:cont, conn, acc} or {:halt, conn, acc}, got: #{inspect(other)}" + end + end + + defp recv_response([{:done, ref} | _], acc, fun, conn, ref, _timeout) do + case fun.(conn, :done, acc) do + {:cont, conn, acc} -> + {:ok, conn, acc} + + {:halt, conn, acc} -> + {:ok, conn, acc} + + other -> + raise ArgumentError, + "expected {:cont, conn, acc} or {:halt, conn, acc}, got: #{inspect(other)}" + end + end + + defp recv_response([], acc, fun, conn, ref, timeout) do + start_time = System.monotonic_time(:millisecond) + + with {:ok, conn, entries} <- recv(conn, 0, timeout) do + timeout = + if is_integer(timeout) do + timeout - System.monotonic_time(:millisecond) - start_time + else + timeout + end + + recv_response(entries, acc, fun, conn, ref, timeout) + end + end + @doc """ See `Mint.HTTP.set_mode/2`. """ diff --git a/test/mint/http1/conn_test.exs b/test/mint/http1/conn_test.exs index 779be260..f66a78ab 100644 --- a/test/mint/http1/conn_test.exs +++ b/test/mint/http1/conn_test.exs @@ -5,9 +5,37 @@ defmodule Mint.HTTP1Test do require Mint.HTTP + test "foo" do + {:ok, _} = + Bandit.start_link( + port: 4000, + plug: fn conn, _ -> + Plug.Conn.send_resp(conn, 200, "hi") + end, + startup_log: false + ) + + {:ok, conn} = HTTP1.connect(:http, "localhost", 4000, mode: :passive) + {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) + {:ok, conn, response} = HTTP1.recv_response(conn, ref, 5000) + + assert %{ + status: 200, + headers: [ + {"date", _}, + {"content-length", "2"}, + {"vary", "accept-encoding"}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], + body: "hi" + } = response + + assert HTTP1.open?(conn) + end + setup do {:ok, port, server_ref} = TestServer.start() - assert {:ok, conn} = HTTP1.connect(:http, "localhost", port) + assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) assert_receive {^server_ref, server_socket} [conn: conn, port: port, server_ref: server_ref, server_socket: server_socket] From b499e0dbb3fca9a7d946373efe7aa6e2a5b44f67 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Wed, 31 Jul 2024 13:33:10 +0200 Subject: [PATCH 02/11] Simplify --- lib/mint/http1.ex | 103 ++++++++-------------------------------------- 1 file changed, 17 insertions(+), 86 deletions(-) diff --git a/lib/mint/http1.ex b/lib/mint/http1.ex index 3447b7d0..3b752c4a 100644 --- a/lib/mint/http1.ex +++ b/lib/mint/http1.ex @@ -548,103 +548,34 @@ defmodule Mint.HTTP1 do } """ def recv_response(conn, ref, timeout) do - recv_response(conn, ref, timeout, %{status: nil, headers: [], body: ""}, fn - conn, {:status, status}, acc -> - {:cont, conn, %{acc | status: status}} - - conn, {:headers, headers}, acc -> - {:cont, conn, update_in(acc.headers, &(&1 ++ headers))} - - conn, {:data, data}, acc -> - {:cont, conn, update_in(acc.body, &(&1 <> data))} - - conn, :done, acc -> - {:cont, conn, acc} - end) + recv_response([], %{status: nil, headers: [], body: ""}, conn, ref, timeout) end - @doc """ - TODO - - ## Examples - - iex> {:ok, conn} = Mint.HTTP1.connect(:https, "httpbin.org", 443, mode: :passive) - iex> {:ok, conn, ref} = Mint.HTTP1.request(conn, "GET", "/user-agent", [], nil) - iex> {:ok, _conn, _acc} = - ...> Mint.HTTP1.recv_response(conn, ref, 5000, nil, fn - ...> conn, entry, acc -> - ...> IO.inspect(entry) - ...> {:cont, conn, acc} - ...> end) - - Outputs: - - {:status, 201} - {:headers, [{"date", ...}, ...]} - {:data, "{\\n \\"user-agent\\": \\"mint/1.6.2\\"\\n}\\n"} - :done - """ - def recv_response(conn, ref, timeout, acc, fun) do - recv_response([], acc, fun, conn, ref, timeout) + defp recv_response([{:status, ref, status} | rest], acc, conn, ref, timeout) do + acc = put_in(acc.status, status) + recv_response(rest, acc, conn, ref, timeout) end - defp recv_response([{:status, ref, status} | rest], acc, fun, conn, ref, timeout) do - case fun.(conn, {:status, status}, acc) do - {:cont, conn, acc} -> - recv_response(rest, acc, fun, conn, ref, timeout) - - {:halt, conn, acc} -> - {:ok, conn, acc} - - other -> - raise ArgumentError, - "expected {:cont, conn, acc} or {:halt, conn, acc}, got: #{inspect(other)}" - end + defp recv_response([{:headers, ref, headers} | rest], acc, conn, ref, timeout) do + acc = update_in(acc.headers, &(&1 ++ headers)) + recv_response(rest, acc, conn, ref, timeout) end - defp recv_response([{:headers, ref, headers} | rest], acc, fun, conn, ref, timeout) do - case fun.(conn, {:headers, headers}, acc) do - {:cont, conn, acc} -> - recv_response(rest, acc, fun, conn, ref, timeout) - - {:halt, conn, acc} -> - {:ok, conn, acc} - - other -> - raise ArgumentError, - "expected {:cont, conn, acc} or {:halt, conn, acc}, got: #{inspect(other)}" - end + defp recv_response([{:data, ref, data} | rest], acc, conn, ref, timeout) do + acc = update_in(acc.body, &(&1 <> data)) + recv_response(rest, acc, conn, ref, timeout) end - defp recv_response([{:data, ref, data} | rest], acc, fun, conn, ref, timeout) do - case fun.(conn, {:data, data}, acc) do - {:cont, conn, acc} -> - recv_response(rest, acc, fun, conn, ref, timeout) - - {:halt, conn, acc} -> - {:ok, conn, acc} - - other -> - raise ArgumentError, - "expected {:cont, conn, acc} or {:halt, conn, acc}, got: #{inspect(other)}" - end + defp recv_response([{:done, ref} | _], acc, conn, ref, _timeout) do + {:ok, conn, acc} end - defp recv_response([{:done, ref} | _], acc, fun, conn, ref, _timeout) do - case fun.(conn, :done, acc) do - {:cont, conn, acc} -> - {:ok, conn, acc} - - {:halt, conn, acc} -> - {:ok, conn, acc} - - other -> - raise ArgumentError, - "expected {:cont, conn, acc} or {:halt, conn, acc}, got: #{inspect(other)}" - end + # Ignore entries from other requests. + defp recv_response([_entry | rest], acc, conn, ref, timeout) do + recv_response(rest, acc, conn, ref, timeout) end - defp recv_response([], acc, fun, conn, ref, timeout) do + defp recv_response([], acc, conn, ref, timeout) do start_time = System.monotonic_time(:millisecond) with {:ok, conn, entries} <- recv(conn, 0, timeout) do @@ -655,7 +586,7 @@ defmodule Mint.HTTP1 do timeout end - recv_response(entries, acc, fun, conn, ref, timeout) + recv_response(entries, acc, conn, ref, timeout) end end From 852d90e57340b671a2b57c5e614e1bff38d45195 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Wed, 31 Jul 2024 18:14:53 +0200 Subject: [PATCH 03/11] Update --- lib/mint/core/util.ex | 3 ++ lib/mint/http.ex | 95 +++++++++++++++++++++++++++++++++++ lib/mint/http1.ex | 61 ---------------------- test/http_test.exs | 2 +- test/mint/http1/conn_test.exs | 52 +++++++++---------- test/mint/http2/conn_test.exs | 45 ++++++++++++++++- 6 files changed, 166 insertions(+), 92 deletions(-) diff --git a/lib/mint/core/util.ex b/lib/mint/core/util.ex index 83c1b96d..e7940683 100644 --- a/lib/mint/core/util.ex +++ b/lib/mint/core/util.ex @@ -3,6 +3,9 @@ defmodule Mint.Core.Util do alias Mint.Types + defguard is_timeout(timeout) + when (is_integer(timeout) and timeout >= 0) or timeout == :infinity + @spec hostname(keyword(), String.t()) :: String.t() def hostname(opts, address) when is_list(opts) do case Keyword.fetch(opts, :hostname) do diff --git a/lib/mint/http.ex b/lib/mint/http.ex index a689e20f..4ab011a9 100644 --- a/lib/mint/http.ex +++ b/lib/mint/http.ex @@ -123,6 +123,7 @@ defmodule Mint.HTTP do alias Mint.{Types, TunnelProxy, UnsafeProxy} alias Mint.Core.{Transport, Util} + require Util @behaviour Mint.Core.Conn @@ -872,6 +873,100 @@ defmodule Mint.HTTP do | {:error, t(), Types.error(), [Types.response()]} def recv(conn, byte_count, timeout), do: conn_module(conn).recv(conn, byte_count, timeout) + @version Mix.Project.config()[:version] + + @doc """ + Receives response from the socket in a blocking way. + + This function receives a response for the request identified by `request_ref` on the connection + `conn`. + + `timeout` is the maximum time to wait before returning an error. + + This function is a convenience for repeatedly calling `Mint.HTTP.recv/3`. The result is either: + + * `{:ok, conn, response}` where `conn` is the updated connection and `response` is a map + with the following keys: + + * `:status` - HTTP response status, an integer. + + * `:headers` - HTTP response headers, a list of tuples `{header_name, header_value}`. + + * `:body` - HTTP response body, a binary. + + * `{:error, conn, reason}` where `conn` is the updated connection and `reason` is the cause + of the error. It is important to store the returned connection over the old connection in + case of errors too, because the state of the connection might change when there are errors + as well. An error when sending a request **does not** necessarily mean that the connection + is closed. Use `open?/1` to verify that the connection is open. + + ## Examples + + iex> {:ok, conn} = Mint.HTTP.connect(:https, "httpbin.org", 443, mode: :passive) + iex> {:ok, conn, request_ref} = Mint.HTTP.request(conn, "GET", "/user-agent", [], nil) + iex> {:ok, _conn, response} = Mint.HTTP.recv_response(conn, request_ref, 5000) + iex> response + %{ + status: 200, + headers: [{"date", ...}, ...], + body: "{\\n \\"user-agent\\": \\"#{@version}\\"\\n}\\n" + } + """ + @spec recv_response(t(), Types.request_ref(), timeout()) :: + {:ok, t(), response} | {:error, Types.error()} + when response: %{ + status: non_neg_integer(), + headers: [{binary(), binary()}], + body: binary() + } + def recv_response(conn, ref, timeout) + when is_reference(ref) and Util.is_timeout(timeout) do + recv_response([], %{status: nil, headers: [], body: ""}, conn, ref, timeout) + end + + defp recv_response([{:status, ref, status} | rest], acc, conn, ref, timeout) do + acc = put_in(acc.status, status) + recv_response(rest, acc, conn, ref, timeout) + end + + defp recv_response([{:headers, ref, headers} | rest], acc, conn, ref, timeout) do + acc = update_in(acc.headers, &(&1 ++ headers)) + recv_response(rest, acc, conn, ref, timeout) + end + + defp recv_response([{:data, ref, data} | rest], acc, conn, ref, timeout) do + acc = update_in(acc.body, &(&1 <> data)) + recv_response(rest, acc, conn, ref, timeout) + end + + defp recv_response([{:done, ref} | _rest], acc, conn, ref, _timeout) do + {:ok, conn, acc} + end + + defp recv_response([{:error, ref, error} | _rest], _acc, conn, ref, _timeout) do + {:error, conn, error} + end + + # Ignore entries from other requests. + defp recv_response([_entry | rest], acc, conn, ref, timeout) do + recv_response(rest, acc, conn, ref, timeout) + end + + defp recv_response([], acc, conn, ref, timeout) do + start_time = System.monotonic_time(:millisecond) + + with {:ok, conn, entries} <- recv(conn, 0, timeout) do + timeout = + if is_integer(timeout) do + timeout - System.monotonic_time(:millisecond) - start_time + else + timeout + end + + recv_response(entries, acc, conn, ref, timeout) + end + end + @doc """ Changes the mode of the underlying socket. diff --git a/lib/mint/http1.ex b/lib/mint/http1.ex index 3b752c4a..3c543d5c 100644 --- a/lib/mint/http1.ex +++ b/lib/mint/http1.ex @@ -529,67 +529,6 @@ defmodule Mint.HTTP1 do "Use Mint.HTTP.set_mode/2 to set the connection to passive mode" end - @doc """ - TODO - - ## Examples - - iex> {:ok, conn} = Mint.HTTP1.connect(:https, "httpbin.org", 443, mode: :passive) - iex> {:ok, conn, ref} = Mint.HTTP1.request(conn, "GET", "/status/user-agent", [], nil) - iex> {:ok, _conn, response} = Mint.HTTP1.recv_response(conn, ref, 5000) - iex> response - %{ - status: 201, - headers: [ - {"date", ...}, - ... - ], - body: "{\\n \\"user-agent\\": \\"mint/1.6.2\\"\\n}\\n" - } - """ - def recv_response(conn, ref, timeout) do - recv_response([], %{status: nil, headers: [], body: ""}, conn, ref, timeout) - end - - defp recv_response([{:status, ref, status} | rest], acc, conn, ref, timeout) do - acc = put_in(acc.status, status) - recv_response(rest, acc, conn, ref, timeout) - end - - defp recv_response([{:headers, ref, headers} | rest], acc, conn, ref, timeout) do - acc = update_in(acc.headers, &(&1 ++ headers)) - recv_response(rest, acc, conn, ref, timeout) - end - - defp recv_response([{:data, ref, data} | rest], acc, conn, ref, timeout) do - acc = update_in(acc.body, &(&1 <> data)) - recv_response(rest, acc, conn, ref, timeout) - end - - defp recv_response([{:done, ref} | _], acc, conn, ref, _timeout) do - {:ok, conn, acc} - end - - # Ignore entries from other requests. - defp recv_response([_entry | rest], acc, conn, ref, timeout) do - recv_response(rest, acc, conn, ref, timeout) - end - - defp recv_response([], acc, conn, ref, timeout) do - start_time = System.monotonic_time(:millisecond) - - with {:ok, conn, entries} <- recv(conn, 0, timeout) do - timeout = - if is_integer(timeout) do - timeout - System.monotonic_time(:millisecond) - start_time - else - timeout - end - - recv_response(entries, acc, conn, ref, timeout) - end - end - @doc """ See `Mint.HTTP.set_mode/2`. """ diff --git a/test/http_test.exs b/test/http_test.exs index 70e307c2..afc9eab4 100644 --- a/test/http_test.exs +++ b/test/http_test.exs @@ -1,4 +1,4 @@ defmodule Mint.HTTPTest do use ExUnit.Case, async: true - doctest Mint.HTTP + doctest Mint.HTTP, except: [recv_response: 3] end diff --git a/test/mint/http1/conn_test.exs b/test/mint/http1/conn_test.exs index f66a78ab..395e5211 100644 --- a/test/mint/http1/conn_test.exs +++ b/test/mint/http1/conn_test.exs @@ -5,37 +5,9 @@ defmodule Mint.HTTP1Test do require Mint.HTTP - test "foo" do - {:ok, _} = - Bandit.start_link( - port: 4000, - plug: fn conn, _ -> - Plug.Conn.send_resp(conn, 200, "hi") - end, - startup_log: false - ) - - {:ok, conn} = HTTP1.connect(:http, "localhost", 4000, mode: :passive) - {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) - {:ok, conn, response} = HTTP1.recv_response(conn, ref, 5000) - - assert %{ - status: 200, - headers: [ - {"date", _}, - {"content-length", "2"}, - {"vary", "accept-encoding"}, - {"cache-control", "max-age=0, private, must-revalidate"} - ], - body: "hi" - } = response - - assert HTTP1.open?(conn) - end - setup do {:ok, port, server_ref} = TestServer.start() - assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) + assert {:ok, conn} = HTTP1.connect(:http, "localhost", port) assert_receive {^server_ref, server_socket} [conn: conn, port: port, server_ref: server_ref, server_socket: server_socket] @@ -488,6 +460,28 @@ defmodule Mint.HTTP1Test do assert responses == [{:status, ref, 200}] end + test "starting a connection in :passive mode and using Mint.HTTP.recv_response/3", + %{port: port, server_ref: server_ref} do + assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) + assert_receive {^server_ref, server_socket} + + {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) + + :ok = :gen_tcp.send(server_socket, "HTTP/1.1 200 OK\r\n") + :ok = :gen_tcp.send(server_socket, "content-type: text/plain\r\n") + :ok = :gen_tcp.send(server_socket, "content-length: 10\r\n\r\n") + :ok = :gen_tcp.send(server_socket, "hello") + :ok = :gen_tcp.send(server_socket, "world") + + assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + + assert response == %{ + body: "helloworld", + headers: [{"content-type", "text/plain"}, {"content-length", "10"}], + status: 200 + } + end + test "changing the connection mode with set_mode/2", %{conn: conn, server_socket: server_socket} do assert_raise ArgumentError, ~r"can't use recv/3", fn -> diff --git a/test/mint/http2/conn_test.exs b/test/mint/http2/conn_test.exs index 41bcd4d3..941f0f10 100644 --- a/test/mint/http2/conn_test.exs +++ b/test/mint/http2/conn_test.exs @@ -1129,7 +1129,11 @@ defmodule Mint.HTTP2Test do hbf: hbf, flags: set_flags(:headers, [:end_headers]) ), - data(stream_id: stream_id, data: "some data", flags: set_flags(:data, [])), + data( + stream_id: stream_id, + data: "some data", + flags: set_flags(:data, []) + ), headers( stream_id: stream_id, hbf: trailer_hbf1, @@ -2083,6 +2087,45 @@ defmodule Mint.HTTP2Test do assert HTTP2.open?(conn) end + @tag connect_options: [mode: :passive] + test "starting a connection with :passive mode and using Mint.HTTP.recv_response/3", %{ + conn: conn + } do + {conn, ref} = open_request(conn) + + assert_recv_frames [headers(stream_id: stream_id)] + + data = + server_encode_frames([ + headers( + stream_id: stream_id, + hbf: + server_encode_headers([ + {":status", "200"}, + {"content-type", "text/plain"}, + {"content-length", "10"} + ]), + flags: set_flags(:headers, [:end_headers]) + ), + data( + stream_id: stream_id, + data: "helloworld", + flags: set_flags(:data, [:end_stream]) + ) + ]) + + :ok = :ssl.send(server_get_socket(), data) + assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + + assert response == %{ + body: "helloworld", + headers: [{"content-type", "text/plain"}, {"content-length", "10"}], + status: 200 + } + + assert HTTP2.open?(conn) + end + test "changing the mode of a connection with set_mode/2", %{conn: conn} do assert_raise ArgumentError, ~r"^can't use recv/3", fn -> HTTP2.recv(conn, 0, 100) From 347d7e801057c98aa4aaeade072b93b58c2bc8ac Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Sat, 3 Aug 2024 21:26:45 +0200 Subject: [PATCH 04/11] Use IO.iodata_to_binary/1 --- lib/mint/http.ex | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/mint/http.ex b/lib/mint/http.ex index 4ab011a9..15b7b6d3 100644 --- a/lib/mint/http.ex +++ b/lib/mint/http.ex @@ -921,26 +921,36 @@ defmodule Mint.HTTP do } def recv_response(conn, ref, timeout) when is_reference(ref) and Util.is_timeout(timeout) do - recv_response([], %{status: nil, headers: [], body: ""}, conn, ref, timeout) + recv_response([], {nil, [], ""}, conn, ref, timeout) end - defp recv_response([{:status, ref, status} | rest], acc, conn, ref, timeout) do - acc = put_in(acc.status, status) - recv_response(rest, acc, conn, ref, timeout) + defp recv_response( + [{:status, ref, new_status} | rest], + {_status, headers, body}, + conn, + ref, + timeout + ) do + recv_response(rest, {new_status, headers, body}, conn, ref, timeout) end - defp recv_response([{:headers, ref, headers} | rest], acc, conn, ref, timeout) do - acc = update_in(acc.headers, &(&1 ++ headers)) - recv_response(rest, acc, conn, ref, timeout) + defp recv_response( + [{:headers, ref, new_headers} | rest], + {status, headers, body}, + conn, + ref, + timeout + ) do + recv_response(rest, {status, headers ++ new_headers, body}, conn, ref, timeout) end - defp recv_response([{:data, ref, data} | rest], acc, conn, ref, timeout) do - acc = update_in(acc.body, &(&1 <> data)) - recv_response(rest, acc, conn, ref, timeout) + defp recv_response([{:data, ref, data} | rest], {status, headers, body}, conn, ref, timeout) do + recv_response(rest, {status, headers, [body, data]}, conn, ref, timeout) end - defp recv_response([{:done, ref} | _rest], acc, conn, ref, _timeout) do - {:ok, conn, acc} + defp recv_response([{:done, ref} | _rest], {status, headers, body}, conn, ref, _timeout) do + response = %{status: status, headers: headers, body: IO.iodata_to_binary(body)} + {:ok, conn, response} end defp recv_response([{:error, ref, error} | _rest], _acc, conn, ref, _timeout) do From e14016aa6bf8ec081b8c0368a299bb6605b3e888 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Sat, 3 Aug 2024 21:39:15 +0200 Subject: [PATCH 05/11] Add more tests --- lib/mint/http.ex | 20 ++++--- test/mint/http1/conn_test.exs | 60 ++++++++++++++------ test/mint/http2/conn_test.exs | 100 +++++++++++++++++++++------------- 3 files changed, 115 insertions(+), 65 deletions(-) diff --git a/lib/mint/http.ex b/lib/mint/http.ex index 15b7b6d3..54be9497 100644 --- a/lib/mint/http.ex +++ b/lib/mint/http.ex @@ -965,15 +965,19 @@ defmodule Mint.HTTP do defp recv_response([], acc, conn, ref, timeout) do start_time = System.monotonic_time(:millisecond) - with {:ok, conn, entries} <- recv(conn, 0, timeout) do - timeout = - if is_integer(timeout) do - timeout - System.monotonic_time(:millisecond) - start_time - else - timeout - end + case recv(conn, 0, timeout) do + {:ok, conn, entries} -> + timeout = + if is_integer(timeout) do + timeout - System.monotonic_time(:millisecond) - start_time + else + timeout + end + + recv_response(entries, acc, conn, ref, timeout) - recv_response(entries, acc, conn, ref, timeout) + {:error, conn, reason, _responses} -> + {:error, conn, reason} end end diff --git a/test/mint/http1/conn_test.exs b/test/mint/http1/conn_test.exs index 395e5211..7d646459 100644 --- a/test/mint/http1/conn_test.exs +++ b/test/mint/http1/conn_test.exs @@ -460,26 +460,50 @@ defmodule Mint.HTTP1Test do assert responses == [{:status, ref, 200}] end - test "starting a connection in :passive mode and using Mint.HTTP.recv_response/3", - %{port: port, server_ref: server_ref} do - assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) - assert_receive {^server_ref, server_socket} + describe "Mint.HTTP.recv_response/3" do + test "receives response", %{port: port, server_ref: server_ref} do + assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) + assert_receive {^server_ref, server_socket} - {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) + {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) - :ok = :gen_tcp.send(server_socket, "HTTP/1.1 200 OK\r\n") - :ok = :gen_tcp.send(server_socket, "content-type: text/plain\r\n") - :ok = :gen_tcp.send(server_socket, "content-length: 10\r\n\r\n") - :ok = :gen_tcp.send(server_socket, "hello") - :ok = :gen_tcp.send(server_socket, "world") - - assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) - - assert response == %{ - body: "helloworld", - headers: [{"content-type", "text/plain"}, {"content-length", "10"}], - status: 200 - } + :ok = :gen_tcp.send(server_socket, "HTTP/1.1 200 OK\r\n") + :ok = :gen_tcp.send(server_socket, "content-type: text/plain\r\n") + :ok = :gen_tcp.send(server_socket, "content-length: 10\r\n\r\n") + :ok = :gen_tcp.send(server_socket, "hello") + :ok = :gen_tcp.send(server_socket, "world") + + assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + + assert response == %{ + body: "helloworld", + headers: [{"content-type", "text/plain"}, {"content-length", "10"}], + status: 200 + } + end + + test "handles errors", %{port: port, server_ref: server_ref} do + assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) + assert_receive {^server_ref, server_socket} + + {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) + + :ok = :gen_tcp.send(server_socket, "HTTP/1.1 200 OK\r\n") + :ok = :gen_tcp.close(server_socket) + + assert {:error, _conn, %Mint.TransportError{reason: :closed}} = + Mint.HTTP.recv_response(conn, ref, 100) + end + + test "handles timeout", %{port: port, server_ref: server_ref} do + assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) + assert_receive {^server_ref, _server_socket} + + {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) + + assert {:error, _conn, %Mint.TransportError{reason: :timeout}} = + Mint.HTTP.recv_response(conn, ref, 1) + end end test "changing the connection mode with set_mode/2", diff --git a/test/mint/http2/conn_test.exs b/test/mint/http2/conn_test.exs index 941f0f10..a948e166 100644 --- a/test/mint/http2/conn_test.exs +++ b/test/mint/http2/conn_test.exs @@ -2087,45 +2087,6 @@ defmodule Mint.HTTP2Test do assert HTTP2.open?(conn) end - @tag connect_options: [mode: :passive] - test "starting a connection with :passive mode and using Mint.HTTP.recv_response/3", %{ - conn: conn - } do - {conn, ref} = open_request(conn) - - assert_recv_frames [headers(stream_id: stream_id)] - - data = - server_encode_frames([ - headers( - stream_id: stream_id, - hbf: - server_encode_headers([ - {":status", "200"}, - {"content-type", "text/plain"}, - {"content-length", "10"} - ]), - flags: set_flags(:headers, [:end_headers]) - ), - data( - stream_id: stream_id, - data: "helloworld", - flags: set_flags(:data, [:end_stream]) - ) - ]) - - :ok = :ssl.send(server_get_socket(), data) - assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) - - assert response == %{ - body: "helloworld", - headers: [{"content-type", "text/plain"}, {"content-length", "10"}], - status: 200 - } - - assert HTTP2.open?(conn) - end - test "changing the mode of a connection with set_mode/2", %{conn: conn} do assert_raise ArgumentError, ~r"^can't use recv/3", fn -> HTTP2.recv(conn, 0, 100) @@ -2206,6 +2167,67 @@ defmodule Mint.HTTP2Test do end end + describe "Mint.HTTP.recv_response/3" do + @describetag connect_options: [mode: :passive] + + test "receives response", %{conn: conn} do + {conn, ref} = open_request(conn) + + assert_recv_frames [headers(stream_id: stream_id)] + + data = + server_encode_frames([ + headers( + stream_id: stream_id, + hbf: + server_encode_headers([ + {":status", "200"}, + {"content-type", "text/plain"}, + {"content-length", "10"} + ]), + flags: set_flags(:headers, [:end_headers]) + ), + data( + stream_id: stream_id, + data: "helloworld", + flags: set_flags(:data, [:end_stream]) + ) + ]) + + :ok = :ssl.send(server_get_socket(), data) + assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + + assert response == %{ + body: "helloworld", + headers: [{"content-type", "text/plain"}, {"content-length", "10"}], + status: 200 + } + + assert HTTP2.open?(conn) + end + + test "handles errors", %{conn: conn} do + {conn, ref} = open_request(conn) + assert_recv_frames [headers(stream_id: _stream_id)] + :ok = :ssl.close(server_get_socket()) + + assert {:error, _conn, %Mint.TransportError{reason: :closed}} = + Mint.HTTP.recv_response(conn, ref, 100) + + assert HTTP2.open?(conn) + end + + test "handles timeout", %{conn: conn} do + {conn, ref} = open_request(conn) + assert_recv_frames [headers(stream_id: _stream_id)] + + assert {:error, _conn, %Mint.TransportError{reason: :timeout}} = + Mint.HTTP.recv_response(conn, ref, 1) + + assert HTTP2.open?(conn) + end + end + describe "ping" do test "if we send a PING we then get a :pong reply", %{conn: conn} do assert {:ok, conn, ref} = HTTP2.ping(conn) From 2c450a756559681eecb8afb3987f43d71a8cb5d2 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Sat, 3 Aug 2024 21:44:25 +0200 Subject: [PATCH 06/11] Improve docs --- lib/mint/http.ex | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/mint/http.ex b/lib/mint/http.ex index 54be9497..4283f1c4 100644 --- a/lib/mint/http.ex +++ b/lib/mint/http.ex @@ -900,6 +900,9 @@ defmodule Mint.HTTP do as well. An error when sending a request **does not** necessarily mean that the connection is closed. Use `open?/1` to verify that the connection is open. + Contrary to `recv/3`, this function does not return partial responses on errors. Use + `recv/3` for full control. + ## Examples iex> {:ok, conn} = Mint.HTTP.connect(:https, "httpbin.org", 443, mode: :passive) From 2c10fca77a161582014e09444f1a0de67d802261 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Sat, 3 Aug 2024 21:57:33 +0200 Subject: [PATCH 07/11] Improve docs and tests --- lib/mint/http.ex | 9 ++++--- test/mint/http1/conn_test.exs | 26 +++++++++++++++++++ test/mint/http2/conn_test.exs | 49 +++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/lib/mint/http.ex b/lib/mint/http.ex index 4283f1c4..639fff5c 100644 --- a/lib/mint/http.ex +++ b/lib/mint/http.ex @@ -903,6 +903,9 @@ defmodule Mint.HTTP do Contrary to `recv/3`, this function does not return partial responses on errors. Use `recv/3` for full control. + This function only handles responses for the given `request_ref`. Do not use it when making + concurrent requests as responses other than for `request_ref` are ignored. + ## Examples iex> {:ok, conn} = Mint.HTTP.connect(:https, "httpbin.org", 443, mode: :passive) @@ -922,9 +925,9 @@ defmodule Mint.HTTP do headers: [{binary(), binary()}], body: binary() } - def recv_response(conn, ref, timeout) - when is_reference(ref) and Util.is_timeout(timeout) do - recv_response([], {nil, [], ""}, conn, ref, timeout) + def recv_response(conn, request_ref, timeout) + when is_reference(request_ref) and Util.is_timeout(timeout) do + recv_response([], {nil, [], ""}, conn, request_ref, timeout) end defp recv_response( diff --git a/test/mint/http1/conn_test.exs b/test/mint/http1/conn_test.exs index 7d646459..507110f5 100644 --- a/test/mint/http1/conn_test.exs +++ b/test/mint/http1/conn_test.exs @@ -482,6 +482,32 @@ defmodule Mint.HTTP1Test do } end + test "handles trailers", %{port: port, server_ref: server_ref} do + assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) + assert_receive {^server_ref, server_socket} + + {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) + + :ok = :gen_tcp.send(server_socket, "HTTP/1.1 200 OK\r\n") + :ok = :gen_tcp.send(server_socket, "transfer-encoding: chunked\r\n") + :ok = :gen_tcp.send(server_socket, "trailer: x-trailer\r\n\r\n") + :ok = :gen_tcp.send(server_socket, "5\r\nhello\r\n") + :ok = :gen_tcp.send(server_socket, "5\r\nworld\r\n0\r\n") + :ok = :gen_tcp.send(server_socket, "x-trailer: foo\r\n\r\n") + + assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + + assert response == %{ + body: "helloworld", + headers: [ + {"transfer-encoding", "chunked"}, + {"trailer", "x-trailer"}, + {"x-trailer", "foo"} + ], + status: 200 + } + end + test "handles errors", %{port: port, server_ref: server_ref} do assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) assert_receive {^server_ref, server_socket} diff --git a/test/mint/http2/conn_test.exs b/test/mint/http2/conn_test.exs index a948e166..212d165b 100644 --- a/test/mint/http2/conn_test.exs +++ b/test/mint/http2/conn_test.exs @@ -2206,6 +2206,55 @@ defmodule Mint.HTTP2Test do assert HTTP2.open?(conn) end + test "handles trailers", %{conn: conn} do + {conn, ref} = open_request(conn) + + assert_recv_frames [headers(stream_id: stream_id)] + + <> = + server_encode_headers([{"x-trailer", "foo"}]) + + data = + server_encode_frames([ + headers( + stream_id: stream_id, + hbf: + server_encode_headers([ + {":status", "200"}, + {"content-type", "text/plain"} + ]), + flags: set_flags(:headers, [:end_headers]) + ), + data( + stream_id: stream_id, + data: "helloworld", + flags: set_flags(:data, []) + ), + headers( + stream_id: stream_id, + hbf: trailer_hbf1, + flags: set_flags(:headers, [:end_stream]) + ), + continuation( + stream_id: stream_id, + hbf: trailer_hbf2, + flags: set_flags(:continuation, [:end_headers]) + ) + ]) + + :ok = :ssl.send(server_get_socket(), data) + + assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + + assert response == %{ + body: "helloworld", + headers: [{"content-type", "text/plain"}, {"x-trailer", "foo"}], + status: 200 + } + + assert HTTP2.open?(conn) + end + test "handles errors", %{conn: conn} do {conn, ref} = open_request(conn) assert_recv_frames [headers(stream_id: _stream_id)] From 90142af2a9f28409a044d48756a27c92f338ddef Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Wed, 14 Aug 2024 15:40:54 +0200 Subject: [PATCH 08/11] Rename to `Mint.HTTP.request_and_response/6` --- lib/mint/http.ex | 43 +++++--- test/http_test.exs | 2 +- test/mint/http1/conn_test.exs | 24 ++--- test/mint/http2/conn_test.exs | 188 ++++++++++++++++++++-------------- 4 files changed, 152 insertions(+), 105 deletions(-) diff --git a/lib/mint/http.ex b/lib/mint/http.ex index 639fff5c..2021fc7e 100644 --- a/lib/mint/http.ex +++ b/lib/mint/http.ex @@ -876,14 +876,11 @@ defmodule Mint.HTTP do @version Mix.Project.config()[:version] @doc """ - Receives response from the socket in a blocking way. + Sends a request and receives a response from the socket in a blocking way. - This function receives a response for the request identified by `request_ref` on the connection - `conn`. + This function is a convenience for sending a request with `request/5` and repeatedly calling `recv/3`. - `timeout` is the maximum time to wait before returning an error. - - This function is a convenience for repeatedly calling `Mint.HTTP.recv/3`. The result is either: + The result is either: * `{:ok, conn, response}` where `conn` is the updated connection and `response` is a map with the following keys: @@ -903,30 +900,48 @@ defmodule Mint.HTTP do Contrary to `recv/3`, this function does not return partial responses on errors. Use `recv/3` for full control. - This function only handles responses for the given `request_ref`. Do not use it when making - concurrent requests as responses other than for `request_ref` are ignored. + ## Options + + * `:timeout` - the maximum amount of time in milliseconds waiting to receive the response. + Setting to `:infinity`, disables the timeout. Defaults to `:infinity`. ## Examples iex> {:ok, conn} = Mint.HTTP.connect(:https, "httpbin.org", 443, mode: :passive) - iex> {:ok, conn, request_ref} = Mint.HTTP.request(conn, "GET", "/user-agent", [], nil) - iex> {:ok, _conn, response} = Mint.HTTP.recv_response(conn, request_ref, 5000) + iex> {:ok, conn, response} = Mint.HTTP.request_and_response(conn, "GET", "/user-agent", [], nil) iex> response %{ status: 200, headers: [{"date", ...}, ...], body: "{\\n \\"user-agent\\": \\"#{@version}\\"\\n}\\n" } + iex> Mint.HTTP.open?(conn) + true """ - @spec recv_response(t(), Types.request_ref(), timeout()) :: - {:ok, t(), response} | {:error, Types.error()} + @spec request_and_response( + t(), + method :: String.t(), + path :: String.t(), + Types.headers(), + body :: iodata() | nil | :stream, + options :: [{:timeout, timeout()}] + ) :: + {:ok, t(), response} + | {:error, t(), Types.error()} when response: %{ status: non_neg_integer(), headers: [{binary(), binary()}], body: binary() } - def recv_response(conn, request_ref, timeout) - when is_reference(request_ref) and Util.is_timeout(timeout) do + def request_and_response(conn, method, path, headers, body, options \\ []) do + options = Keyword.validate!(options, timeout: :infinity) + + with {:ok, conn, ref} <- request(conn, method, path, headers, body) do + recv_response(conn, ref, options[:timeout]) + end + end + + defp recv_response(conn, request_ref, timeout) when Util.is_timeout(timeout) do recv_response([], {nil, [], ""}, conn, request_ref, timeout) end diff --git a/test/http_test.exs b/test/http_test.exs index afc9eab4..c88d4fd3 100644 --- a/test/http_test.exs +++ b/test/http_test.exs @@ -1,4 +1,4 @@ defmodule Mint.HTTPTest do use ExUnit.Case, async: true - doctest Mint.HTTP, except: [recv_response: 3] + doctest Mint.HTTP, except: [request_and_response: 6] end diff --git a/test/mint/http1/conn_test.exs b/test/mint/http1/conn_test.exs index 507110f5..0c7ec945 100644 --- a/test/mint/http1/conn_test.exs +++ b/test/mint/http1/conn_test.exs @@ -460,34 +460,32 @@ defmodule Mint.HTTP1Test do assert responses == [{:status, ref, 200}] end - describe "Mint.HTTP.recv_response/3" do + describe "Mint.HTTP.request_and_response/6" do test "receives response", %{port: port, server_ref: server_ref} do assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) assert_receive {^server_ref, server_socket} - {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) - :ok = :gen_tcp.send(server_socket, "HTTP/1.1 200 OK\r\n") :ok = :gen_tcp.send(server_socket, "content-type: text/plain\r\n") :ok = :gen_tcp.send(server_socket, "content-length: 10\r\n\r\n") :ok = :gen_tcp.send(server_socket, "hello") :ok = :gen_tcp.send(server_socket, "world") - assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + {:ok, conn, response} = Mint.HTTP.request_and_response(conn, "GET", "/", [], nil) assert response == %{ body: "helloworld", headers: [{"content-type", "text/plain"}, {"content-length", "10"}], status: 200 } + + assert HTTP1.open?(conn) end test "handles trailers", %{port: port, server_ref: server_ref} do assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) assert_receive {^server_ref, server_socket} - {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) - :ok = :gen_tcp.send(server_socket, "HTTP/1.1 200 OK\r\n") :ok = :gen_tcp.send(server_socket, "transfer-encoding: chunked\r\n") :ok = :gen_tcp.send(server_socket, "trailer: x-trailer\r\n\r\n") @@ -495,7 +493,7 @@ defmodule Mint.HTTP1Test do :ok = :gen_tcp.send(server_socket, "5\r\nworld\r\n0\r\n") :ok = :gen_tcp.send(server_socket, "x-trailer: foo\r\n\r\n") - assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + {:ok, conn, response} = Mint.HTTP.request_and_response(conn, "GET", "/", [], nil) assert response == %{ body: "helloworld", @@ -506,29 +504,29 @@ defmodule Mint.HTTP1Test do ], status: 200 } + + assert HTTP1.open?(conn) end test "handles errors", %{port: port, server_ref: server_ref} do assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) assert_receive {^server_ref, server_socket} - {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) - :ok = :gen_tcp.send(server_socket, "HTTP/1.1 200 OK\r\n") :ok = :gen_tcp.close(server_socket) assert {:error, _conn, %Mint.TransportError{reason: :closed}} = - Mint.HTTP.recv_response(conn, ref, 100) + Mint.HTTP.request_and_response(conn, "GET", "/", [], nil) end test "handles timeout", %{port: port, server_ref: server_ref} do assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) assert_receive {^server_ref, _server_socket} - {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil) + assert {:error, conn, %Mint.TransportError{reason: :timeout}} = + Mint.HTTP.request_and_response(conn, "GET", "/", [], nil, timeout: 0) - assert {:error, _conn, %Mint.TransportError{reason: :timeout}} = - Mint.HTTP.recv_response(conn, ref, 1) + refute HTTP1.open?(conn) end end diff --git a/test/mint/http2/conn_test.exs b/test/mint/http2/conn_test.exs index 212d165b..16b2f85d 100644 --- a/test/mint/http2/conn_test.exs +++ b/test/mint/http2/conn_test.exs @@ -2167,35 +2167,44 @@ defmodule Mint.HTTP2Test do end end - describe "Mint.HTTP.recv_response/3" do + describe "Mint.HTTP.request_and_response/6" do @describetag connect_options: [mode: :passive] - test "receives response", %{conn: conn} do - {conn, ref} = open_request(conn) - - assert_recv_frames [headers(stream_id: stream_id)] - - data = - server_encode_frames([ - headers( - stream_id: stream_id, - hbf: - server_encode_headers([ - {":status", "200"}, - {"content-type", "text/plain"}, - {"content-length", "10"} - ]), - flags: set_flags(:headers, [:end_headers]) - ), - data( - stream_id: stream_id, - data: "helloworld", - flags: set_flags(:data, [:end_stream]) - ) - ]) + test "receives response", context do + pid = self() + + Task.start_link(fn -> + context = Map.merge(context, start_server_async(context)) + [conn: conn] = start_connection(context) + send(pid, {:conn, conn}) + + assert_recv_frames [headers(stream_id: stream_id)] + + data = + server_encode_frames([ + headers( + stream_id: stream_id, + hbf: + server_encode_headers([ + {":status", "200"}, + {"content-type", "text/plain"}, + {"content-length", "10"} + ]), + flags: set_flags(:headers, [:end_headers]) + ), + data( + stream_id: stream_id, + data: "helloworld", + flags: set_flags(:data, [:end_stream]) + ) + ]) + + :ok = :ssl.send(server_get_socket(), data) + Process.sleep(:infinity) + end) - :ok = :ssl.send(server_get_socket(), data) - assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + assert_receive {:conn, conn} + assert {:ok, conn, response} = Mint.HTTP.request_and_response(conn, "GET", "/", [], nil) assert response == %{ body: "helloworld", @@ -2206,45 +2215,53 @@ defmodule Mint.HTTP2Test do assert HTTP2.open?(conn) end - test "handles trailers", %{conn: conn} do - {conn, ref} = open_request(conn) - - assert_recv_frames [headers(stream_id: stream_id)] - - <> = - server_encode_headers([{"x-trailer", "foo"}]) - - data = - server_encode_frames([ - headers( - stream_id: stream_id, - hbf: - server_encode_headers([ - {":status", "200"}, - {"content-type", "text/plain"} - ]), - flags: set_flags(:headers, [:end_headers]) - ), - data( - stream_id: stream_id, - data: "helloworld", - flags: set_flags(:data, []) - ), - headers( - stream_id: stream_id, - hbf: trailer_hbf1, - flags: set_flags(:headers, [:end_stream]) - ), - continuation( - stream_id: stream_id, - hbf: trailer_hbf2, - flags: set_flags(:continuation, [:end_headers]) - ) - ]) - - :ok = :ssl.send(server_get_socket(), data) + test "handles trailers", context do + pid = self() + + Task.start_link(fn -> + context = Map.merge(context, start_server_async(context)) + [conn: conn] = start_connection(context) + send(pid, {:conn, conn}) + + assert_recv_frames [headers(stream_id: stream_id)] + + <> = + server_encode_headers([{"x-trailer", "foo"}]) + + data = + server_encode_frames([ + headers( + stream_id: stream_id, + hbf: + server_encode_headers([ + {":status", "200"}, + {"content-type", "text/plain"} + ]), + flags: set_flags(:headers, [:end_headers]) + ), + data( + stream_id: stream_id, + data: "helloworld", + flags: set_flags(:data, []) + ), + headers( + stream_id: stream_id, + hbf: trailer_hbf1, + flags: set_flags(:headers, [:end_stream]) + ), + continuation( + stream_id: stream_id, + hbf: trailer_hbf2, + flags: set_flags(:continuation, [:end_headers]) + ) + ]) + + :ok = :ssl.send(server_get_socket(), data) + Process.sleep(:infinity) + end) - assert {:ok, _conn, response} = Mint.HTTP.recv_response(conn, ref, 100) + assert_receive {:conn, conn} + assert {:ok, conn, response} = Mint.HTTP.request_and_response(conn, "GET", "/", [], nil) assert response == %{ body: "helloworld", @@ -2255,25 +2272,42 @@ defmodule Mint.HTTP2Test do assert HTTP2.open?(conn) end - test "handles errors", %{conn: conn} do - {conn, ref} = open_request(conn) - assert_recv_frames [headers(stream_id: _stream_id)] - :ok = :ssl.close(server_get_socket()) + test "handles errors", context do + pid = self() - assert {:error, _conn, %Mint.TransportError{reason: :closed}} = - Mint.HTTP.recv_response(conn, ref, 100) + Task.start_link(fn -> + context = Map.merge(context, start_server_async(context)) + [conn: conn] = start_connection(context) + send(pid, {:conn, conn}) - assert HTTP2.open?(conn) + assert_recv_frames [headers(stream_id: _stream_id)] + :ok = :ssl.close(server_get_socket()) + end) + + assert_receive {:conn, conn} + + assert {:error, conn, %Mint.TransportError{reason: :closed}} = + Mint.HTTP.request_and_response(conn, "GET", "/", [], nil) + + refute HTTP2.open?(conn) end - test "handles timeout", %{conn: conn} do - {conn, ref} = open_request(conn) - assert_recv_frames [headers(stream_id: _stream_id)] + test "handles timeout", context do + pid = self() - assert {:error, _conn, %Mint.TransportError{reason: :timeout}} = - Mint.HTTP.recv_response(conn, ref, 1) + Task.start_link(fn -> + context = Map.merge(context, start_server_async(context)) + [conn: conn] = start_connection(context) + send(pid, {:conn, conn}) + Process.sleep(:infinity) + end) - assert HTTP2.open?(conn) + assert_receive {:conn, conn} + + assert {:error, conn, %Mint.TransportError{reason: :timeout}} = + Mint.HTTP.request_and_response(conn, "GET", "/", [], nil, timeout: 0) + + refute HTTP2.open?(conn) end end From 7dde3bbb81db5d7cf1ce8a78eb2ad347f7bf9ac5 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Wed, 14 Aug 2024 16:34:15 +0200 Subject: [PATCH 09/11] Update docs & test --- lib/mint/http.ex | 10 ++++++++-- test/mint/http1/conn_test.exs | 22 ++++++++++++++++++++++ test/support/mint/http1/test_server.ex | 12 ++++++++---- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/lib/mint/http.ex b/lib/mint/http.ex index 2021fc7e..d0009662 100644 --- a/lib/mint/http.ex +++ b/lib/mint/http.ex @@ -900,6 +900,11 @@ defmodule Mint.HTTP do Contrary to `recv/3`, this function does not return partial responses on errors. Use `recv/3` for full control. + > #### Error {: .error} + > + > This function can only be used for one-off requests. If there is another concurrent request, + > started by `request/5`, it will crash. + ## Options * `:timeout` - the maximum amount of time in milliseconds waiting to receive the response. @@ -979,8 +984,9 @@ defmodule Mint.HTTP do end # Ignore entries from other requests. - defp recv_response([_entry | rest], acc, conn, ref, timeout) do - recv_response(rest, acc, conn, ref, timeout) + defp recv_response([entry | _rest], _acc, _conn, _ref, _timeout) when is_tuple(entry) do + ref = elem(entry, 1) + raise "received unexpected response from request #{inspect(ref)}" end defp recv_response([], acc, conn, ref, timeout) do diff --git a/test/mint/http1/conn_test.exs b/test/mint/http1/conn_test.exs index 0c7ec945..0f765010 100644 --- a/test/mint/http1/conn_test.exs +++ b/test/mint/http1/conn_test.exs @@ -528,6 +528,28 @@ defmodule Mint.HTTP1Test do refute HTTP1.open?(conn) end + + test "raises on multiple requests " do + {:ok, port, server_ref} = + TestServer.start(fn %{socket: socket} -> + :ok = :gen_tcp.send(socket, "HTTP/1.1 200 OK\r\n") + :ok = :gen_tcp.send(socket, "content-type: text/plain\r\n") + :ok = :gen_tcp.send(socket, "content-length: 10\r\n\r\n") + :ok = :gen_tcp.send(socket, "hello") + :ok = :gen_tcp.send(socket, "world") + end) + + assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, mode: :passive) + assert_receive {^server_ref, _server_socket} + + {:ok, conn, ref} = Mint.HTTP.request(conn, "GET", "/", [], nil) + + assert_raise RuntimeError, + "received unexpected response from request #{inspect(ref)}", + fn -> + Mint.HTTP.request_and_response(conn, "GET", "/", [], nil) + end + end end test "changing the connection mode with set_mode/2", diff --git a/test/support/mint/http1/test_server.ex b/test/support/mint/http1/test_server.ex index 79632fa0..1a1e2533 100644 --- a/test/support/mint/http1/test_server.ex +++ b/test/support/mint/http1/test_server.ex @@ -1,17 +1,17 @@ defmodule Mint.HTTP1.TestServer do - def start do + def start(fun \\ nil) do {:ok, listen_socket} = :gen_tcp.listen(0, mode: :binary, packet: :raw) server_ref = make_ref() parent = self() - spawn_link(fn -> loop(listen_socket, parent, server_ref) end) + spawn_link(fn -> loop(listen_socket, parent, server_ref, fun) end) with {:ok, port} <- :inet.port(listen_socket) do {:ok, port, server_ref} end end - defp loop(listen_socket, parent, server_ref) do + defp loop(listen_socket, parent, server_ref, fun) do case :gen_tcp.accept(listen_socket) do {:ok, socket} -> send(parent, {server_ref, socket}) @@ -22,7 +22,11 @@ defmodule Mint.HTTP1.TestServer do {:error, :einval} -> :ok end - loop(listen_socket, parent, server_ref) + if fun do + fun.(%{socket: socket, parent: parent}) + end + + loop(listen_socket, parent, server_ref, fun) {:error, :closed} -> :ok From f307e26068a73967b7242a707f34db762fe22367 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Wed, 14 Aug 2024 16:40:11 +0200 Subject: [PATCH 10/11] Fix tests --- lib/mint/http.ex | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/mint/http.ex b/lib/mint/http.ex index d0009662..10fb76db 100644 --- a/lib/mint/http.ex +++ b/lib/mint/http.ex @@ -939,7 +939,7 @@ defmodule Mint.HTTP do body: binary() } def request_and_response(conn, method, path, headers, body, options \\ []) do - options = Keyword.validate!(options, timeout: :infinity) + options = keyword_validate!(options, timeout: :infinity) with {:ok, conn, ref} <- request(conn, method, path, headers, body) do recv_response(conn, ref, options[:timeout]) @@ -1008,6 +1008,13 @@ defmodule Mint.HTTP do end end + # TODO: Remove when we require Elixir v1.13 + if Version.match?(System.version(), ">= 1.13.0") do + defp keyword_validate!(keyword, values), do: Keyword.validate!(keyword, values) + else + defp keyword_validate!(keyword, _values), do: keyword + end + @doc """ Changes the mode of the underlying socket. From 55ab096f5279d6b3e29ddcd2654eda50680b0c15 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Thu, 15 Aug 2024 10:07:21 +0200 Subject: [PATCH 11/11] Apply suggestions from code review Co-authored-by: Andrea Leopardi --- lib/mint/http.ex | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/mint/http.ex b/lib/mint/http.ex index 10fb76db..e04871b7 100644 --- a/lib/mint/http.ex +++ b/lib/mint/http.ex @@ -922,6 +922,7 @@ defmodule Mint.HTTP do } iex> Mint.HTTP.open?(conn) true + """ @spec request_and_response( t(), @@ -934,8 +935,8 @@ defmodule Mint.HTTP do {:ok, t(), response} | {:error, t(), Types.error()} when response: %{ - status: non_neg_integer(), - headers: [{binary(), binary()}], + status: Types.status(), + headers: Types.headers(), body: binary() } def request_and_response(conn, method, path, headers, body, options \\ []) do @@ -1009,7 +1010,7 @@ defmodule Mint.HTTP do end # TODO: Remove when we require Elixir v1.13 - if Version.match?(System.version(), ">= 1.13.0") do + if function_exported?(Keyword, :validate!, 2) do defp keyword_validate!(keyword, values), do: Keyword.validate!(keyword, values) else defp keyword_validate!(keyword, _values), do: keyword