From 18e12ed5eaf1e286b0ef5ab561bc8b847a89a17b Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Wed, 22 Nov 2023 14:52:44 +0100 Subject: [PATCH 1/2] Fix Bandit.HTTP1.Adapter.inform/3 return value Per https://hexdocs.pm/plug/Plug.Conn.Adapter.html#c:inform/3, the callback should return: :ok | {:error, :not_supported} The way Plug.Conn.inform! is implemented at the moment, https://github.com/elixir-plug/plug/blob/v1.15.2/lib/plug/conn.ex#L1335, it looked like Bandit did _not_ implement inform but it sure does! --- lib/bandit/http1/adapter.ex | 15 +++------------ test/bandit/http1/request_test.exs | 7 ++++++- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/lib/bandit/http1/adapter.ex b/lib/bandit/http1/adapter.ex index a43cb9ac..d5c87b4e 100644 --- a/lib/bandit/http1/adapter.ex +++ b/lib/bandit/http1/adapter.ex @@ -505,19 +505,10 @@ defmodule Bandit.HTTP1.Adapter do @impl Plug.Conn.Adapter def inform(%__MODULE__{version: :"HTTP/1.0"}, _status, _headers), do: {:error, :not_supported} - def inform(%__MODULE__{socket: socket, version: version} = req, status, headers) do - start_time = Bandit.Telemetry.monotonic_time() - - {header_iodata, header_metrics} = response_header(version, status, headers) + def inform(%__MODULE__{socket: socket, version: version}, status, headers) do + {header_iodata, _header_metrics} = response_header(version, status, headers) _ = ThousandIsland.Socket.send(socket, header_iodata) - - metrics = - req.metrics - |> Map.merge(header_metrics) - |> Map.put(:resp_start_time, start_time) - |> Map.put(:resp_body_bytes, 0) - - {:ok, nil, %{req | metrics: metrics}} + :ok end defp response_header(nil, status, headers), do: response_header("HTTP/1.0", status, headers) diff --git a/test/bandit/http1/request_test.exs b/test/bandit/http1/request_test.exs index 770f9832..fbe16a9f 100644 --- a/test/bandit/http1/request_test.exs +++ b/test/bandit/http1/request_test.exs @@ -1336,7 +1336,7 @@ defmodule HTTP1RequestTest do test "sending informational responses", context do client = SimpleHTTP1Client.tcp_client(context) - SimpleHTTP1Client.send(client, "GET", "/send_inform", ["host: localhost"]) + SimpleHTTP1Client.send(client, "GET", "/send_inform!", ["host: localhost"]) Process.sleep(100) assert {:ok, "100 Continue", headers, rest} = SimpleHTTP1Client.recv_reply(client) @@ -1356,6 +1356,11 @@ defmodule HTTP1RequestTest do conn |> send_resp(200, "Informer") end + def send_inform!(conn) do + conn = conn |> inform!(100, [{"x-from", "inform"}]) + conn |> send_resp(200, "Informer") + end + test "reading HTTP version", context do response = Req.get!(context.req, url: "/report_version") From 5645795a428adb6b00649b7d13146f293dc45c11 Mon Sep 17 00:00:00 2001 From: Wojtek Mach Date: Thu, 23 Nov 2023 11:21:26 +0100 Subject: [PATCH 2/2] Update --- lib/bandit/http1/adapter.ex | 15 ++++++++++++--- test/bandit/http1/request_test.exs | 7 +------ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/bandit/http1/adapter.ex b/lib/bandit/http1/adapter.ex index d5c87b4e..a4155528 100644 --- a/lib/bandit/http1/adapter.ex +++ b/lib/bandit/http1/adapter.ex @@ -505,10 +505,19 @@ defmodule Bandit.HTTP1.Adapter do @impl Plug.Conn.Adapter def inform(%__MODULE__{version: :"HTTP/1.0"}, _status, _headers), do: {:error, :not_supported} - def inform(%__MODULE__{socket: socket, version: version}, status, headers) do - {header_iodata, _header_metrics} = response_header(version, status, headers) + def inform(%__MODULE__{socket: socket, version: version} = req, status, headers) do + start_time = Bandit.Telemetry.monotonic_time() + + {header_iodata, header_metrics} = response_header(version, status, headers) _ = ThousandIsland.Socket.send(socket, header_iodata) - :ok + + metrics = + req.metrics + |> Map.merge(header_metrics) + |> Map.put(:resp_start_time, start_time) + |> Map.put(:resp_body_bytes, 0) + + {:ok, %{req | metrics: metrics}} end defp response_header(nil, status, headers), do: response_header("HTTP/1.0", status, headers) diff --git a/test/bandit/http1/request_test.exs b/test/bandit/http1/request_test.exs index fbe16a9f..770f9832 100644 --- a/test/bandit/http1/request_test.exs +++ b/test/bandit/http1/request_test.exs @@ -1336,7 +1336,7 @@ defmodule HTTP1RequestTest do test "sending informational responses", context do client = SimpleHTTP1Client.tcp_client(context) - SimpleHTTP1Client.send(client, "GET", "/send_inform!", ["host: localhost"]) + SimpleHTTP1Client.send(client, "GET", "/send_inform", ["host: localhost"]) Process.sleep(100) assert {:ok, "100 Continue", headers, rest} = SimpleHTTP1Client.recv_reply(client) @@ -1356,11 +1356,6 @@ defmodule HTTP1RequestTest do conn |> send_resp(200, "Informer") end - def send_inform!(conn) do - conn = conn |> inform!(100, [{"x-from", "inform"}]) - conn |> send_resp(200, "Informer") - end - test "reading HTTP version", context do response = Req.get!(context.req, url: "/report_version")