From e8eeb1815046378ca099f9a94dc16b16555a06c3 Mon Sep 17 00:00:00 2001 From: Mat Trudel Date: Mon, 9 Oct 2023 09:51:28 -0400 Subject: [PATCH] Fix handling of HEAD responses (#242) * Record HTTP/1 method in req * Support HEAD, 204, 304 quelling for HTTP/1 responses * Refactor h2 send_file logic Also fix file always being closed after h2 sendfile file call, as in https://github.com/mtrudel/thousand_island/pull/78 * Record HTTP/2 method in req * Add content-length header to HTTP/2 sendfile responses * Support HEAD, 204, 304 quelling for HTTP/2 responses --- lib/bandit/http1/adapter.ex | 55 ++++++--- lib/bandit/http1/handler.ex | 12 +- lib/bandit/http2/adapter.ex | 75 +++++++----- lib/bandit/http2/stream_task.ex | 3 +- test/bandit/http1/request_test.exs | 81 +++++++++++-- test/bandit/http2/plug_test.exs | 1 + test/bandit/http2/protocol_test.exs | 173 +++++++++++++++++++++++++++- test/support/simple_http1_client.ex | 9 +- 8 files changed, 345 insertions(+), 64 deletions(-) diff --git a/lib/bandit/http1/adapter.ex b/lib/bandit/http1/adapter.ex index 638aa3f8..a43cb9ac 100644 --- a/lib/bandit/http1/adapter.ex +++ b/lib/bandit/http1/adapter.ex @@ -14,6 +14,7 @@ defmodule Bandit.HTTP1.Adapter do body_remaining: nil, body_encoding: nil, version: nil, + method: nil, keepalive: false, content_encoding: nil, upgrade: nil, @@ -31,6 +32,7 @@ defmodule Bandit.HTTP1.Adapter do body_remaining: nil | integer(), body_encoding: nil | binary(), version: nil | :"HTTP/1.1" | :"HTTP/1.0", + method: Plug.Conn.method() | nil, keepalive: boolean(), content_encoding: String.t(), upgrade: nil | {:websocket, opts :: keyword(), websocket_opts :: keyword()}, @@ -46,22 +48,23 @@ defmodule Bandit.HTTP1.Adapter do # Header Reading ################ - def read_request_line(req, method \\ nil, request_target \\ nil) do + def read_request_line(req, request_target \\ nil) do packet_size = Keyword.get(req.opts.http_1, :max_request_line_length, 10_000) case :erlang.decode_packet(:http_bin, req.buffer, packet_size: packet_size) do {:more, _len} -> with {:ok, chunk} <- read_available(req.socket, _read_timeout = nil) do - read_request_line(%{req | buffer: req.buffer <> chunk}, method, request_target) + read_request_line(%{req | buffer: req.buffer <> chunk}, request_target) end {:ok, {:http_request, method, request_target, version}, rest} -> with {:ok, version} <- get_version(version), {:ok, request_target} <- resolve_request_target(request_target) do + method = to_string(method) bytes_read = byte_size(req.buffer) - byte_size(rest) metrics = Map.update(req.metrics, :req_line_bytes, bytes_read, &(&1 + bytes_read)) - req = %{req | buffer: rest, version: version, metrics: metrics} - {:ok, to_string(method), request_target, req} + req = %{req | buffer: rest, version: version, method: method, metrics: metrics} + {:ok, request_target, req} end {:ok, {:http_error, reason}, _rest} -> @@ -360,7 +363,7 @@ defmodule Bandit.HTTP1.Adapter do def send_resp(%__MODULE__{write_state: :chunking_out}, _, _, _), do: raise(Plug.Conn.AlreadySentError) - def send_resp(%__MODULE__{socket: socket, version: version} = req, status, headers, body) do + def send_resp(%__MODULE__{} = req, status, headers, body) do start_time = Bandit.Telemetry.monotonic_time() response_content_encoding_header = Bandit.Headers.get_header(headers, "content-encoding") @@ -403,14 +406,22 @@ defmodule Bandit.HTTP1.Adapter do body_bytes = IO.iodata_length(body) headers = Bandit.Headers.add_content_length(headers, body_bytes, status) - {header_iodata, header_metrics} = response_header(version, status, headers) - _ = ThousandIsland.Socket.send(socket, [header_iodata, body]) + {header_iodata, header_metrics} = response_header(req.version, status, headers) + + {body_iodata, body_metrics} = + if send_resp_body?(req, status) do + {body, %{resp_body_bytes: body_bytes}} + else + {[], %{resp_body_bytes: 0}} + end + + _ = ThousandIsland.Socket.send(req.socket, [header_iodata | body_iodata]) metrics = req.metrics |> Map.merge(compression_metrics) |> Map.merge(header_metrics) - |> Map.put(:resp_body_bytes, body_bytes) + |> Map.merge(body_metrics) |> Map.put(:resp_start_time, start_time) |> Map.put(:resp_end_time, Bandit.Telemetry.monotonic_time()) @@ -436,15 +447,22 @@ defmodule Bandit.HTTP1.Adapter do end if offset + length <= size do - headers = [{"content-length", length |> to_string()} | headers] + headers = Bandit.Headers.add_content_length(headers, length, status) {header_iodata, header_metrics} = response_header(version, status, headers) _ = ThousandIsland.Socket.send(socket, header_iodata) - _ = ThousandIsland.Socket.sendfile(socket, path, offset, length) + + body_metrics = + if send_resp_body?(req, status) do + _ = ThousandIsland.Socket.sendfile(socket, path, offset, length) + %{resp_body_bytes: length} + else + %{resp_body_bytes: 0} + end metrics = req.metrics |> Map.merge(header_metrics) - |> Map.put(:resp_body_bytes, length) + |> Map.merge(body_metrics) |> Map.put(:resp_start_time, start_time) |> Map.put(:resp_end_time, Bandit.Telemetry.monotonic_time()) @@ -469,15 +487,21 @@ defmodule Bandit.HTTP1.Adapter do |> Map.put(:resp_start_time, start_time) |> Map.put(:resp_body_bytes, 0) - {:ok, nil, %{req | write_state: :chunking_out, metrics: metrics}} + if send_resp_body?(req, status) do + {:ok, nil, %{req | write_state: :chunking_out, metrics: metrics}} + else + {:ok, nil, %{req | write_state: :sent, metrics: metrics}} + end end @impl Plug.Conn.Adapter - def chunk(%__MODULE__{socket: socket}, chunk) do + def chunk(%__MODULE__{write_state: :chunking_out, socket: socket}, chunk) do byte_size = chunk |> IO.iodata_length() |> Integer.to_string(16) ThousandIsland.Socket.send(socket, [byte_size, "\r\n", chunk, "\r\n"]) end + def chunk(_, _), do: :ok + @impl Plug.Conn.Adapter def inform(%__MODULE__{version: :"HTTP/1.0"}, _status, _headers), do: {:error, :not_supported} @@ -525,6 +549,11 @@ defmodule Bandit.HTTP1.Adapter do {[resp_line, headers], metrics} end + defp send_resp_body?(%{method: "HEAD"}, _status), do: false + defp send_resp_body?(_req, 204), do: false + defp send_resp_body?(_req, 304), do: false + defp send_resp_body?(_req, _status), do: true + @impl Plug.Conn.Adapter def upgrade(%Bandit.HTTP1.Adapter{websocket_enabled: true} = req, :websocket, opts), do: {:ok, %{req | upgrade: {:websocket, opts, req.opts.websocket}}} diff --git a/lib/bandit/http1/handler.ex b/lib/bandit/http1/handler.ex index a442d4e2..f9cf51d6 100644 --- a/lib/bandit/http1/handler.ex +++ b/lib/bandit/http1/handler.ex @@ -22,16 +22,16 @@ defmodule Bandit.HTTP1.Handler do with {:ok, transport_info} <- Bandit.TransportInfo.init(socket), req <- %{req | transport_info: transport_info}, - {:ok, method, request_target, req} <- Bandit.HTTP1.Adapter.read_request_line(req) do + {:ok, request_target, req} <- Bandit.HTTP1.Adapter.read_request_line(req) do try do with {:ok, headers, req} <- Bandit.HTTP1.Adapter.read_headers(req), {:ok, :no_upgrade} <- - maybe_upgrade_h2c(state, req, transport_info, method, request_target, headers), + maybe_upgrade_h2c(state, req, transport_info, req.method, request_target, headers), {:ok, %Plug.Conn{adapter: {Bandit.HTTP1.Adapter, req}} = conn} <- Bandit.Pipeline.run( {Bandit.HTTP1.Adapter, req}, transport_info, - method, + req.method, request_target, headers, state.plug @@ -39,7 +39,7 @@ defmodule Bandit.HTTP1.Handler do Bandit.Telemetry.stop_span(span, req.metrics, %{ conn: conn, status: conn.status, - method: method, + method: req.method, request_target: request_target }) @@ -52,7 +52,7 @@ defmodule Bandit.HTTP1.Handler do Bandit.Telemetry.stop_span(span, %{}, %{ error: reason, status: code, - method: method, + method: req.method, request_target: request_target }) @@ -62,7 +62,7 @@ defmodule Bandit.HTTP1.Handler do Bandit.Telemetry.stop_span(span, req.metrics, %{ conn: conn, status: conn.status, - method: method, + method: req.method, request_target: request_target }) diff --git a/lib/bandit/http2/adapter.ex b/lib/bandit/http2/adapter.ex index bf891125..32852659 100644 --- a/lib/bandit/http2/adapter.ex +++ b/lib/bandit/http2/adapter.ex @@ -10,6 +10,7 @@ defmodule Bandit.HTTP2.Adapter do transport_info: nil, stream_id: nil, end_stream: false, + method: nil, content_encoding: nil, metrics: %{}, opts: [] @@ -20,6 +21,7 @@ defmodule Bandit.HTTP2.Adapter do transport_info: Bandit.TransportInfo.t(), stream_id: Bandit.HTTP2.Stream.stream_id(), end_stream: boolean(), + method: Plug.Conn.method() | nil, content_encoding: String.t() | nil, metrics: map(), opts: keyword() @@ -147,7 +149,7 @@ defmodule Bandit.HTTP2.Adapter do headers = Bandit.Headers.add_content_length(headers, body_bytes, status) adapter = - if body_bytes == 0 do + if body_bytes == 0 || !send_resp_body?(adapter, status) do adapter |> send_headers(status, headers, true) else @@ -168,46 +170,54 @@ defmodule Bandit.HTTP2.Adapter do def send_file(%__MODULE__{} = adapter, status, headers, path, offset, length) do %File.Stat{type: :regular, size: size} = File.stat!(path) length = if length == :all, do: size - offset, else: length + headers = Bandit.Headers.add_content_length(headers, length, status) - cond do - offset + length == size && offset == 0 -> - adapter = send_headers(adapter, status, headers, false) + adapter = + cond do + !send_resp_body?(adapter, status) -> + send_headers(adapter, status, headers, true) + + offset + length == size && offset == 0 -> + adapter = send_headers(adapter, status, headers, false) - adapter = path |> File.stream!([], 2048) |> Enum.reduce(adapter, fn chunk, adapter -> send_data(adapter, chunk, false) end) |> send_data(<<>>, true) - metrics = - adapter.metrics - |> Map.put(:resp_end_time, Bandit.Telemetry.monotonic_time()) - - {:ok, nil, %{adapter | metrics: metrics}} - - offset + length < size -> - with {:ok, fd} <- :file.open(path, [:raw, :binary]), - {:ok, data} <- :file.pread(fd, offset, length) do - adapter = - adapter - |> send_headers(status, headers, false) - |> send_data(data, true) - - metrics = - adapter.metrics - |> Map.put(:resp_end_time, Bandit.Telemetry.monotonic_time()) + offset + length < size -> + case :file.open(path, [:raw, :binary]) do + {:ok, fd} -> + try do + with {:ok, data} <- :file.pread(fd, offset, length) do + adapter + |> send_headers(status, headers, false) + |> send_data(data, true) + end + after + :file.close(fd) + end + + {:error, reason} -> + {:error, reason} + end + + true -> + raise "Cannot read #{length} bytes starting at #{offset} as #{path} is only #{size} octets in length" + end - {:ok, nil, %{adapter | metrics: metrics}} - end + metrics = Map.put(adapter.metrics, :resp_end_time, Bandit.Telemetry.monotonic_time()) - true -> - raise "Cannot read #{length} bytes starting at #{offset} as #{path} is only #{size} octets in length" - end + {:ok, nil, %{adapter | metrics: metrics}} end @impl Plug.Conn.Adapter def send_chunked(%__MODULE__{} = adapter, status, headers) do - {:ok, nil, send_headers(adapter, status, headers, false)} + if send_resp_body?(adapter, status) do + {:ok, nil, send_headers(adapter, status, headers, false)} + else + {:ok, nil, send_headers(adapter, status, headers, true)} + end end @impl Plug.Conn.Adapter @@ -217,6 +227,10 @@ defmodule Bandit.HTTP2.Adapter do # details) and closing the stream here carves closest to the underlying HTTP/1.1 behaviour # (RFC9112§7.1). The whole notion of chunked encoding is moot in HTTP/2 anyway (RFC9113§8.1) # so this entire section of the API is a bit slanty regardless. + # + # Moreover, if the caller is chunking out on a HEAD, 204 or 304 response, the underlying + # stream will have been closed in send_chunked/3 above, and so this call will return an + # `{:error, :not_owner}` error here (which we ignore, but it's still kinda odd) _ = send_data(adapter, chunk, IO.iodata_length(chunk) == 0) :ok end @@ -242,6 +256,11 @@ defmodule Bandit.HTTP2.Adapter do @impl Plug.Conn.Adapter def get_http_protocol(%__MODULE__{}), do: :"HTTP/2" + defp send_resp_body?(%{method: "HEAD"}, _status), do: false + defp send_resp_body?(_req, 204), do: false + defp send_resp_body?(_req, 304), do: false + defp send_resp_body?(_req, _status), do: true + defp send_headers(adapter, status, headers, end_stream) do metrics = adapter.metrics diff --git a/lib/bandit/http2/stream_task.ex b/lib/bandit/http2/stream_task.ex index f8542be6..cead2d4f 100644 --- a/lib/bandit/http2/stream_task.ex +++ b/lib/bandit/http2/stream_task.ex @@ -52,7 +52,8 @@ defmodule Bandit.HTTP2.StreamTask do def run(req, transport_info, all_headers, plug, span) do with {:ok, request_target} <- build_request_target(all_headers), - method <- Bandit.Headers.get_header(all_headers, ":method") do + method <- Bandit.Headers.get_header(all_headers, ":method"), + req <- %{req | method: method} do with {:ok, pseudo_headers, headers} <- split_headers(all_headers), :ok <- pseudo_headers_all_request(pseudo_headers), :ok <- exactly_one_instance_of(pseudo_headers, ":scheme"), diff --git a/test/bandit/http1/request_test.exs b/test/bandit/http1/request_test.exs index cbb757ca..770f9832 100644 --- a/test/bandit/http1/request_test.exs +++ b/test/bandit/http1/request_test.exs @@ -1107,6 +1107,14 @@ defmodule HTTP1RequestTest do |> send_resp(200, String.duplicate("a", 10_000)) end + test "sends expected content-length but no body for HEAD requests", context do + client = SimpleHTTP1Client.tcp_client(context) + SimpleHTTP1Client.send(client, "HEAD", "/send_big_body", ["host: localhost"]) + + assert {:ok, "200 OK", headers, ""} = SimpleHTTP1Client.recv_reply(client, true) + assert Bandit.Headers.get_header(headers, :"content-length") == "10000" + end + test "replaces any incorrect provided content-length headers", context do response = Req.get!(context.req, url: "/send_incorrect_content_length") @@ -1121,28 +1129,30 @@ defmodule HTTP1RequestTest do |> send_resp(200, String.duplicate("a", 10_000)) end - test "writes out a response with no content-length header for 204 responses", context do - response = Req.get!(context.req, url: "/send_204") + test "writes out a response with no content-length header or body for 204 responses", + context do + client = SimpleHTTP1Client.tcp_client(context) + SimpleHTTP1Client.send(client, "GET", "/send_204", ["host: localhost"]) - assert response.status == 204 - assert response.body == "" - assert response.headers["content-length"] == nil + assert {:ok, "204 No Content", headers, ""} = SimpleHTTP1Client.recv_reply(client) + assert Bandit.Headers.get_header(headers, :"content-length") == nil end def send_204(conn) do - send_resp(conn, 204, "") + send_resp(conn, 204, "this is an invalid body") end - test "writes out a response with no content-length header for 304 responses", context do - response = Req.get!(context.req, url: "/send_304") + test "writes out a response with no content-length header or body for 304 responses", + context do + client = SimpleHTTP1Client.tcp_client(context) + SimpleHTTP1Client.send(client, "GET", "/send_304", ["host: localhost"]) - assert response.status == 304 - assert response.body == "" - assert response.headers["content-length"] == nil + assert {:ok, "304 Not Modified", headers, ""} = SimpleHTTP1Client.recv_reply(client) + assert Bandit.Headers.get_header(headers, :"content-length") == nil end def send_304(conn) do - send_resp(conn, 304, "") + send_resp(conn, 304, "this is an invalid body") end test "writes out a response with zero content-length for 200 responses", context do @@ -1198,6 +1208,14 @@ defmodule HTTP1RequestTest do conn end + test "does not write out a body for a chunked response to a HEAD request", context do + client = SimpleHTTP1Client.tcp_client(context) + SimpleHTTP1Client.send(client, "HEAD", "/send_chunked_200", ["host: localhost"]) + + assert {:ok, "200 OK", headers, ""} = SimpleHTTP1Client.recv_reply(client, true) + assert Bandit.Headers.get_header(headers, :"transfer-encoding") == "chunked" + end + test "writes out a chunked iolist response", context do response = Req.get!(context.req, url: "/send_chunked_200_iolist") @@ -1251,11 +1269,50 @@ defmodule HTTP1RequestTest do assert response.headers["content-length"] == ["6"] end + test "writes out headers but not body for files requested via HEAD request", context do + client = SimpleHTTP1Client.tcp_client(context) + SimpleHTTP1Client.send(client, "HEAD", "/send_full_file", ["host: localhost"]) + + assert {:ok, "200 OK", headers, ""} = SimpleHTTP1Client.recv_reply(client, true) + assert Bandit.Headers.get_header(headers, :"content-length") == "6" + assert SimpleHTTP1Client.connection_closed_for_reading?(client) + end + def send_full_file(conn) do conn |> send_file(200, Path.join([__DIR__, "../../support/sendfile"]), 0, :all) end + test "does not write out a content-length header or body for files on a 204", + context do + client = SimpleHTTP1Client.tcp_client(context) + SimpleHTTP1Client.send(client, "HEAD", "/send_full_file_204", ["host: localhost"]) + + assert {:ok, "204 No Content", headers, ""} = SimpleHTTP1Client.recv_reply(client, true) + assert Bandit.Headers.get_header(headers, :"content-length") == nil + assert SimpleHTTP1Client.connection_closed_for_reading?(client) + end + + def send_full_file_204(conn) do + conn + |> send_file(204, Path.join([__DIR__, "../../support/sendfile"]), 0, :all) + end + + test "does not write out a content-length header or body for files on a 304", + context do + client = SimpleHTTP1Client.tcp_client(context) + SimpleHTTP1Client.send(client, "HEAD", "/send_full_file_304", ["host: localhost"]) + + assert {:ok, "304 Not Modified", headers, ""} = SimpleHTTP1Client.recv_reply(client, true) + assert Bandit.Headers.get_header(headers, :"content-length") == nil + assert SimpleHTTP1Client.connection_closed_for_reading?(client) + end + + def send_full_file_304(conn) do + conn + |> send_file(304, Path.join([__DIR__, "../../support/sendfile"]), 0, :all) + end + test "writes out a sent file for parts of a file with content length", context do response = Req.get!(context.req, url: "/send_file?offset=1&length=3") diff --git a/test/bandit/http2/plug_test.exs b/test/bandit/http2/plug_test.exs index c03d93ce..553ad39f 100644 --- a/test/bandit/http2/plug_test.exs +++ b/test/bandit/http2/plug_test.exs @@ -286,6 +286,7 @@ defmodule HTTP2PlugTest do response = Req.get!(context.req, url: "/send_full_file") assert response.status == 200 + assert response.headers["content-length"] == ["6"] assert response.body == "ABCDEF" end diff --git a/test/bandit/http2/protocol_test.exs b/test/bandit/http2/protocol_test.exs index 7a79cf6e..44db116b 100644 --- a/test/bandit/http2/protocol_test.exs +++ b/test/bandit/http2/protocol_test.exs @@ -518,6 +518,21 @@ defmodule HTTP2ProtocolTest do |> send_resp(200, String.duplicate("a", 10_000)) end + test "sends expected content-length but no body for HEAD requests", context do + socket = SimpleH2Client.setup_connection(context) + + SimpleH2Client.send_simple_headers(socket, 1, :head, "/send_big_body", context[:port]) + + assert {:ok, 1, true, + [ + {":status", "200"}, + {"date", _date}, + {"content-length", "10000"}, + {"vary", "accept-encoding"}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], _ctx} = SimpleH2Client.recv_headers(socket) + end + test "replaces any incorrect provided content-length headers", context do context = https_server(context, http_2_options: [compress: false]) @@ -548,6 +563,103 @@ defmodule HTTP2ProtocolTest do |> send_resp(200, String.duplicate("a", 10_000)) end + test "sends no content-length header or body for 204 responses", context do + socket = SimpleH2Client.setup_connection(context) + + SimpleH2Client.send_simple_headers(socket, 1, :get, "/send_204", context[:port]) + + assert {:ok, 1, true, + [ + {":status", "204"}, + {"date", _date}, + {"vary", "accept-encoding"}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], _ctx} = SimpleH2Client.recv_headers(socket) + end + + def send_204(conn) do + send_resp(conn, 204, "this is an invalid body") + end + + test "sends no content-length header or body for 304 responses", context do + socket = SimpleH2Client.setup_connection(context) + + SimpleH2Client.send_simple_headers(socket, 1, :get, "/send_304", context[:port]) + + assert {:ok, 1, true, + [ + {":status", "304"}, + {"date", _date}, + {"vary", "accept-encoding"}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], _ctx} = SimpleH2Client.recv_headers(socket) + end + + def send_304(conn) do + send_resp(conn, 304, "this is an invalid body") + end + + test "sends headers but no body for a HEAD request to a file", context do + socket = SimpleH2Client.setup_connection(context) + + SimpleH2Client.send_simple_headers(socket, 1, :head, "/send_file", context.port) + + assert {:ok, 1, true, + [ + {":status", "200"}, + {"date", _date}, + {"content-length", "6"}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], _ctx} = SimpleH2Client.recv_headers(socket) + + assert SimpleH2Client.recv_goaway_and_close(socket) == {:ok, 1, 0} + end + + def send_file(conn) do + conn + |> send_file(200, Path.join([__DIR__, "../../support/sendfile"]), 0, :all) + end + + test "sends no content-length header or body for a 204 request to a file", context do + socket = SimpleH2Client.setup_connection(context) + + SimpleH2Client.send_simple_headers(socket, 1, :get, "/send_file_204", context.port) + + assert {:ok, 1, true, + [ + {":status", "204"}, + {"date", _date}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], _ctx} = SimpleH2Client.recv_headers(socket) + + assert SimpleH2Client.recv_goaway_and_close(socket) == {:ok, 1, 0} + end + + def send_file_204(conn) do + conn + |> send_file(204, Path.join([__DIR__, "../../support/sendfile"]), 0, :all) + end + + test "writes out headers but no body for a 304 request to a file", context do + socket = SimpleH2Client.setup_connection(context) + + SimpleH2Client.send_simple_headers(socket, 1, :get, "/send_file_304", context.port) + + assert {:ok, 1, true, + [ + {":status", "304"}, + {"date", _date}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], _ctx} = SimpleH2Client.recv_headers(socket) + + assert SimpleH2Client.recv_goaway_and_close(socket) == {:ok, 1, 0} + end + + def send_file_304(conn) do + conn + |> send_file(304, Path.join([__DIR__, "../../support/sendfile"]), 0, :all) + end + test "sends multiple DATA frames with last one end of stream when chunking", context do socket = SimpleH2Client.setup_connection(context) @@ -559,6 +671,21 @@ defmodule HTTP2ProtocolTest do assert SimpleH2Client.recv_body(socket) == {:ok, 1, true, ""} end + test "does not write out a body for a chunked response to a HEAD request", context do + socket = SimpleH2Client.setup_connection(context) + + SimpleH2Client.send_simple_headers(socket, 1, :head, "/chunk_response", context.port) + + assert {:ok, 1, true, + [ + {":status", "200"}, + {"date", _date}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], _ctx} = SimpleH2Client.recv_headers(socket) + + assert SimpleH2Client.recv_goaway_and_close(socket) == {:ok, 1, 0} + end + def chunk_response(conn) do conn |> send_chunked(200) @@ -568,6 +695,50 @@ defmodule HTTP2ProtocolTest do |> elem(1) end + test "does not write out a body for a chunked 204 response", context do + socket = SimpleH2Client.setup_connection(context) + + SimpleH2Client.send_simple_headers(socket, 1, :get, "/chunk_204", context[:port]) + + assert {:ok, 1, true, + [ + {":status", "204"}, + {"date", _date}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], _ctx} = SimpleH2Client.recv_headers(socket) + + assert SimpleH2Client.recv_goaway_and_close(socket) == {:ok, 1, 0} + end + + def chunk_204(conn) do + conn + |> send_chunked(204) + |> chunk("This is invalid") + |> elem(1) + end + + test "does not write out a body for a chunked 304 response", context do + socket = SimpleH2Client.setup_connection(context) + + SimpleH2Client.send_simple_headers(socket, 1, :get, "/chunk_304", context[:port]) + + assert {:ok, 1, true, + [ + {":status", "304"}, + {"date", _date}, + {"cache-control", "max-age=0, private, must-revalidate"} + ], _ctx} = SimpleH2Client.recv_headers(socket) + + assert SimpleH2Client.recv_goaway_and_close(socket) == {:ok, 1, 0} + end + + def chunk_304(conn) do + conn + |> send_chunked(304) + |> chunk("This is invalid") + |> elem(1) + end + test "sends multiple DATA frames when sending iolist chunks", context do socket = SimpleH2Client.setup_connection(context) @@ -918,7 +1089,7 @@ defmodule HTTP2ProtocolTest do def headers_for_header_read_test(context) do headers = [ - {":method", "HEAD"}, + {":method", "GET"}, {":path", "/header_read_test"}, {":scheme", "https"}, {":authority", "localhost:#{context.port}"}, diff --git a/test/support/simple_http1_client.ex b/test/support/simple_http1_client.ex index 4bf39537..2e5ed632 100644 --- a/test/support/simple_http1_client.ex +++ b/test/support/simple_http1_client.ex @@ -9,12 +9,12 @@ defmodule SimpleHTTP1Client do Transport.send(socket, "\r\n") end - def recv_reply(socket) do + def recv_reply(socket, head? \\ false) do {:ok, response} = Transport.recv(socket, 0) - parse_response(socket, response) + parse_response(socket, response, head?) end - def parse_response(socket, response) do + def parse_response(socket, response, head? \\ false) do [status_line | headers] = String.split(response, "\r\n") <<_version::binary-size(8), " ", status::binary>> = status_line {headers, rest} = Enum.split_while(headers, &(&1 != "")) @@ -31,6 +31,9 @@ defmodule SimpleHTTP1Client do headers |> Keyword.get(:"content-length") |> case do + _ when head? -> + rest + nil -> rest