Support HEAD, 204, 304 quelling for HTTP/2 responses

lib/bandit/http2/adapter.ex

@@ -149,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
@@ -174,6 +174,9 @@ defmodule Bandit.HTTP2.Adapter do
 
     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)
 
@@ -201,7 +204,11 @@ defmodule Bandit.HTTP2.Adapter do
 
   @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
@@ -211,6 +218,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
@@ -236,6 +247,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

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}"},