RFC Conformance: Enforce HTTP/2 trailer field section requirements

I had Claude check the code against various RFC conformance, and it flagged below. Claude helped with the desc.

## Summary

Validate request trailers as an HTTP/2 field section and require the trailer HEADERS frame to carry `END_STREAM`.

## What was not conformant

Bandit accepted a second request HEADERS frame without `END_STREAM`, logged and ignored its fields, and continued reading DATA. It also checked only for pseudo-headers in trailers, so the normal HTTP/2 rules for lowercase names, field values, connection-specific fields, and `TE` were not applied.

[RFC 9113 section 8.1](https://www.rfc-editor.org/rfc/rfc9113.html#section-8.1) says a trailer field section must terminate the stream. It specifically requires `END_STREAM` on the terminating HEADERS frame and says a trailer section without it is a malformed request. [RFC 9113 section 8.2.1](https://www.rfc-editor.org/rfc/rfc9113.html#section-8.2.1), [RFC 9110 section 5.1](https://www.rfc-editor.org/rfc/rfc9110.html#section-5.1), and [RFC 9113 section 8.2.2](https://www.rfc-editor.org/rfc/rfc9113.html#section-8.2.2) define the field-name, field-value, and connection-specific field rules that apply to HTTP/2 field sections.

## Implementation

- Reject trailer HEADERS unless the frame carries `END_STREAM`.
- Continue rejecting pseudo-headers in trailers.
- Apply Bandit's existing lowercase-name, field-value, connection-specific field, and `TE` validators to trailers before discarding them.
- Enforce the full regular field-name token grammar and the HTTP/2 prohibition on edge SP or HTAB in trailer values.
- Validate every member of every `TE` field line, while accepting repeated `trailers` members.
- Match RFC 9113's exact connection-specific field list without rejecting unrelated extension fields by name.
- Keep the existing behavior of accepting and ignoring a valid trailer section.
- Treat a trailer field value that HPAX decodes as `nil` (an indexed static-table entry with no value) as the empty value it represents, instead of letting it crash validation (elixir-mint/hpax#27).

This is intentionally a focused change. It does not add trailer exposure to Plug and does not alter valid request-body handling.

## Tests

The protocol tests cover:

- acceptance of a valid trailer section with `END_STREAM`, including an unrelated extension field named `trailers`;
- rejection of a trailer section without `END_STREAM` with stream `PROTOCOL_ERROR`;
- rejection of a connection-specific trailer field;
- rejection when any of multiple `TE` field lines contains a value other than `trailers`;
- rejection of invalid trailer field names; and
- rejection of CR and edge whitespace in trailer field values; and
- acceptance of a trailer field encoded as an indexed static-table entry with no value.

Run:

```console
mix test test/bandit/http2/protocol_test.exs
```

Verified against Bandit `main` at `b084b37` with Erlang/OTP 27.3 and Elixir 1.18.4:

- `mix format --check-formatted`
- `MIX_ENV=test mix compile --warnings-as-errors`
- Full non-slow suite: 821 tests pass (the suite's one IPv6 server binding test needs an IPv6-capable host)
- `mix credo --strict` and `mix dialyzer` pass on the combined tree of all nineteen prepared Bandit changes
- h2spec 2.6.0 passes in full on the combined tree
- HTTP/2 protocol suite: 184 tests pass

Author: mpettibone

lib/bandit/http2/stream.ex

@@ -219,15 +219,25 @@ defmodule Bandit.HTTP2.Stream do
     # specific cases by RFC9113§8.2.2. We check those cases in a separate filter
     defp no_connection_headers!(headers, stream) do
       connection_headers =
-        ~w[connection keep-alive proxy-authenticate proxy-authorization proxy-connection trailers transfer-encoding upgrade]
+        ~w[connection keep-alive proxy-connection transfer-encoding upgrade]
 
       if Enum.any?(headers, fn {key, _value} -> key in connection_headers end),
         do: stream_error!("Received connection-specific header", stream)
     end
 
     # RFC9113§8.2.2 - TE header may be present if it contains exactly 'trailers'
     defp valid_te_header!(headers, stream) do
-      if Bandit.Headers.get_header(headers, "te") not in [nil, "trailers"],
+      invalid? =
+        headers
+        |> Enum.filter(fn {name, _value} -> name == "te" end)
+        |> Enum.any?(fn {"te", value} ->
+          case Plug.Conn.Utils.list(value) do
+            [] -> true
+            members -> Enum.any?(members, &(String.downcase(&1, :ascii) != "trailers"))
+          end
+        end)
+
+      if invalid?,
         do: stream_error!("Received invalid TE header", stream)
     end
 
@@ -266,7 +276,6 @@ defmodule Bandit.HTTP2.Stream do
          when state in [:open, :local_closed] do
       case do_recv(stream, timeout) do
         {:headers, trailers, stream} ->
-          no_pseudo_headers!(trailers, stream)
           Logger.warning("Ignoring trailers #{inspect(trailers)}", domain: [:bandit])
           do_read_data(stream, max_bytes, timeout, acc)
 
@@ -297,6 +306,56 @@ defmodule Bandit.HTTP2.Stream do
         do: stream_error!("Received trailers with pseudo headers", stream)
     end
 
+    defp validate_trailers!(_headers, false, stream) do
+      stream_error!("Received trailers without END_STREAM", stream)
+    end
+
+    # RFC9113§8.1 - a trailer field section terminates the stream and is subject
+    # to the same field-name and field-value requirements as other field sections
+    defp validate_trailers!(headers, true, stream) do
+      # HPAX decodes an indexed static-table entry with no value to a nil value; treat it
+      # as the empty binary it represents (elixir-mint/hpax#27)
+      headers =
+        Enum.map(headers, fn
+          {name, nil} -> {name, ""}
+          header -> header
+        end)
+
+      no_pseudo_headers!(headers, stream)
+      headers_all_lowercase!(headers, stream)
+      valid_trailer_field_names!(headers, stream)
+      no_connection_headers!(headers, stream)
+      valid_te_header!(headers, stream)
+      valid_field_values!(headers, stream)
+      valid_trailer_edge_whitespace!(headers, stream)
+    end
+
+    defp valid_trailer_field_names!(headers, stream) do
+      if Enum.any?(headers, fn {key, _value} -> not valid_trailer_field_name?(key) end),
+        do: stream_error!("Received invalid trailer field name (RFC9113§8.2.1)", stream)
+    end
+
+    defp valid_trailer_field_name?(<<>>), do: false
+    defp valid_trailer_field_name?(name), do: valid_trailer_field_name_bytes?(name)
+
+    defp valid_trailer_field_name_bytes?(<<char, rest::binary>>)
+         when char in ?a..?z or char in ?0..?9 or char in ~c"!#$%&'*+-.^_`|~",
+         do: valid_trailer_field_name_bytes?(rest)
+
+    defp valid_trailer_field_name_bytes?(<<_char, _rest::binary>>), do: false
+    defp valid_trailer_field_name_bytes?(<<>>), do: true
+
+    defp valid_trailer_edge_whitespace!(headers, stream) do
+      if Enum.any?(headers, fn
+           {_key, <<>>} ->
+             false
+
+           {_key, value} ->
+             :binary.first(value) in [0x09, 0x20] or :binary.last(value) in [0x09, 0x20]
+         end),
+         do: stream_error!("Field value contains invalid characters (RFC9113§8.2.1)", stream)
+    end
+
     defp do_recv(%@for{state: :idle} = stream, timeout) do
       receive do
         {:bandit, {:headers, headers, end_stream}} ->
@@ -319,6 +378,7 @@ defmodule Bandit.HTTP2.Stream do
          when state in [:open, :local_closed] do
       receive do
         {:bandit, {:headers, headers, end_stream}} ->
+          validate_trailers!(headers, end_stream, stream)
           {:headers, headers, stream |> do_recv_headers() |> do_recv_end_stream(end_stream)}
 
         {:bandit, {:data, data, end_stream}} ->
@@ -560,8 +620,9 @@ defmodule Bandit.HTTP2.Stream do
 
     def ensure_completed(%@for{state: :local_closed} = stream) do
       receive do
-        {:bandit, {:headers, _headers, true}} ->
-          do_recv_end_stream(stream, true)
+        {:bandit, {:headers, headers, end_stream}} ->
+          validate_trailers!(headers, end_stream, stream)
+          do_recv_end_stream(stream, end_stream)
 
         {:bandit, {:data, data, true}} ->
           do_recv_data(stream, data, true) |> do_recv_end_stream(true)

test/bandit/http2/protocol_test.exs

@@ -2073,7 +2073,9 @@ defmodule HTTP2ProtocolTest do
 
       {:ok, ctx} = SimpleH2Client.send_simple_headers(socket, 1, :post, "/echo", context.port)
       SimpleH2Client.send_body(socket, 1, false, "OK")
-      SimpleH2Client.send_headers(socket, 1, true, [{"x-trailer", "trailer"}], ctx)
+      # An extension field named "trailers" is not one of RFC9113§8.2.2's
+      # connection-specific fields and must not be rejected merely by name.
+      SimpleH2Client.send_headers(socket, 1, true, [{"trailers", "extension-value"}], ctx)
 
       {:ok, 0, _} = SimpleH2Client.recv_window_update(socket)
       {:ok, 1, _} = SimpleH2Client.recv_window_update(socket)
@@ -2084,6 +2086,136 @@ defmodule HTTP2ProtocolTest do
       assert SimpleH2Client.connection_alive?(socket)
     end
 
+    @tag :capture_log
+    test "accepts a trailer field encoded as an indexed entry with no value", context do
+      socket = SimpleH2Client.setup_connection(context)
+
+      {:ok, _ctx} = SimpleH2Client.send_simple_headers(socket, 1, :post, "/echo", context.port)
+      SimpleH2Client.send_body(socket, 1, false, "OK")
+
+      # Static-table index 58 (`user-agent`) as an indexed field, which HPAX decodes with
+      # a nil value (elixir-mint/hpax#27); as a trailer it must validate as an empty value
+      SimpleH2Client.send_frame(socket, 1, 0x05, 1, <<0xBA>>)
+
+      {:ok, 0, _} = SimpleH2Client.recv_window_update(socket)
+      {:ok, 1, _} = SimpleH2Client.recv_window_update(socket)
+
+      assert SimpleH2Client.successful_response?(socket, 1, false)
+      assert SimpleH2Client.recv_body(socket) == {:ok, 1, true, "OK"}
+
+      assert SimpleH2Client.connection_alive?(socket)
+    end
+
+    @tag :capture_log
+    test "rejects trailer HEADERS without END_STREAM", context do
+      socket = SimpleH2Client.setup_connection(context)
+
+      {:ok, ctx} = SimpleH2Client.send_simple_headers(socket, 1, :post, "/echo", context.port)
+      SimpleH2Client.send_body(socket, 1, false, "OK")
+
+      {:ok, 0, _} = SimpleH2Client.recv_window_update(socket)
+      {:ok, 1, _} = SimpleH2Client.recv_window_update(socket)
+
+      SimpleH2Client.send_headers(socket, 1, false, [{"x-trailer", "trailer"}], ctx)
+
+      assert SimpleH2Client.recv_rst_stream(socket) == {:ok, 1, 1}
+      assert SimpleH2Client.connection_alive?(socket)
+
+      assert_receive {:log, %{level: :error, msg: {:string, msg}, meta: %{stream_id: 1}}}, 500
+      assert msg == "** (Bandit.HTTP2.Errors.StreamError) Received trailers without END_STREAM"
+    end
+
+    @tag :capture_log
+    test "validates connection-specific fields in trailers", context do
+      socket = SimpleH2Client.setup_connection(context)
+
+      {:ok, ctx} = SimpleH2Client.send_simple_headers(socket, 1, :post, "/echo", context.port)
+      SimpleH2Client.send_body(socket, 1, false, "OK")
+
+      {:ok, 0, _} = SimpleH2Client.recv_window_update(socket)
+      {:ok, 1, _} = SimpleH2Client.recv_window_update(socket)
+
+      SimpleH2Client.send_headers(socket, 1, true, [{"connection", "close"}], ctx)
+
+      assert SimpleH2Client.recv_rst_stream(socket) == {:ok, 1, 1}
+      assert SimpleH2Client.connection_alive?(socket)
+
+      assert_receive {:log, %{level: :error, msg: {:string, msg}, meta: %{stream_id: 1}}}, 500
+      assert msg == "** (Bandit.HTTP2.Errors.StreamError) Received connection-specific header"
+    end
+
+    @tag :capture_log
+    test "validates every TE field line in trailers", context do
+      socket = SimpleH2Client.setup_connection(context)
+
+      {:ok, ctx} = SimpleH2Client.send_simple_headers(socket, 1, :post, "/echo", context.port)
+      SimpleH2Client.send_body(socket, 1, false, "OK")
+
+      {:ok, 0, _} = SimpleH2Client.recv_window_update(socket)
+      {:ok, 1, _} = SimpleH2Client.recv_window_update(socket)
+
+      SimpleH2Client.send_headers(socket, 1, true, [{"te", "trailers"}, {"te", "gzip"}], ctx)
+
+      assert SimpleH2Client.recv_rst_stream(socket) == {:ok, 1, 1}
+      assert SimpleH2Client.connection_alive?(socket)
+
+      assert_receive {:log, %{level: :error, msg: {:string, msg}, meta: %{stream_id: 1}}}, 500
+      assert msg == "** (Bandit.HTTP2.Errors.StreamError) Received invalid TE header"
+    end
+
+    @tag :capture_log
+    test "validates field values in trailers", context do
+      socket = SimpleH2Client.setup_connection(context)
+
+      {:ok, ctx} = SimpleH2Client.send_simple_headers(socket, 1, :post, "/echo", context.port)
+      SimpleH2Client.send_body(socket, 1, false, "OK")
+
+      {:ok, 0, _} = SimpleH2Client.recv_window_update(socket)
+      {:ok, 1, _} = SimpleH2Client.recv_window_update(socket)
+
+      SimpleH2Client.send_headers(socket, 1, true, [{"x-trailer", "bad\rvalue"}], ctx)
+
+      assert SimpleH2Client.recv_rst_stream(socket) == {:ok, 1, 1}
+      assert SimpleH2Client.connection_alive?(socket)
+
+      assert_receive {:log, %{level: :error, msg: {:string, msg}, meta: %{stream_id: 1}}}, 500
+
+      assert msg ==
+               "** (Bandit.HTTP2.Errors.StreamError) Field value contains invalid characters (RFC9113§8.2.1)"
+    end
+
+    @tag :capture_log
+    test "validates regular field-name grammar in trailers", context do
+      socket = SimpleH2Client.setup_connection(context)
+
+      {:ok, ctx} = SimpleH2Client.send_simple_headers(socket, 1, :post, "/echo", context.port)
+      SimpleH2Client.send_body(socket, 1, false, "OK")
+
+      {:ok, 0, _} = SimpleH2Client.recv_window_update(socket)
+      {:ok, 1, _} = SimpleH2Client.recv_window_update(socket)
+
+      SimpleH2Client.send_headers(socket, 1, true, [{"bad/name", "value"}], ctx)
+
+      assert SimpleH2Client.recv_rst_stream(socket) == {:ok, 1, 1}
+      assert SimpleH2Client.connection_alive?(socket)
+    end
+
+    @tag :capture_log
+    test "rejects edge whitespace in trailer field values", context do
+      socket = SimpleH2Client.setup_connection(context)
+
+      {:ok, ctx} = SimpleH2Client.send_simple_headers(socket, 1, :post, "/echo", context.port)
+      SimpleH2Client.send_body(socket, 1, false, "OK")
+
+      {:ok, 0, _} = SimpleH2Client.recv_window_update(socket)
+      {:ok, 1, _} = SimpleH2Client.recv_window_update(socket)
+
+      SimpleH2Client.send_headers(socket, 1, true, [{"x-trailer", "trailing "}], ctx)
+
+      assert SimpleH2Client.recv_rst_stream(socket) == {:ok, 1, 1}
+      assert SimpleH2Client.connection_alive?(socket)
+    end
+
     @tag :capture_log
     test "rejects HEADER frames sent as trailers that contain pseudo headers", context do
       socket = SimpleH2Client.setup_connection(context)