Bound buffered header blocks by the encoded size of the list limit

lib/mint/http2.ex

@@ -2954,22 +2954,40 @@ defmodule Mint.HTTP2 do
   # The header block accumulated from a HEADERS frame and its trailing
   # CONTINUATION frames is buffered (in compressed form) until END_HEADERS
   # arrives. A server can withhold END_HEADERS and stream CONTINUATION frames
-  # indefinitely, so the buffered size is bounded by the locally advertised
-  # SETTINGS_MAX_HEADER_LIST_SIZE. Empty fragments are not retained in the
-  # accumulator. The compressed accumulator is never larger than the
-  # uncompressed header list it decodes to, so the size limit never rejects a
-  # header block that fits within the advertised limit.
+  # indefinitely, so the buffered fragments are bounded by the largest encoding
+  # of a header list within the locally advertised SETTINGS_MAX_HEADER_LIST_SIZE,
+  # which is enforced on the decoded list. The fragment that carries END_HEADERS
+  # isn't buffered or counted. Empty fragments are not retained in the
+  # accumulator.
+  #
+  # The bound assumes minimal integer representations (RFC 7541 5.1) and at most
+  # two dynamic table size updates (RFC 7541 4.2). HPACK decoders also accept
+  # redundant zero continuation bytes in integers and any number of size updates,
+  # which no finite bound covers. A Huffman-coded octet takes at most 30 bits
+  # (RFC 7541 Appendix B), and a field's representation byte, integers (at most 6
+  # bytes each below 2^32) and Huffman padding take less than 15 bytes, so a
+  # field encodes to less than the (name + value + 32) * 30 / 8 bytes that its
+  # decoded size (RFC 9113 6.5.2) allows. A size update takes at most 6 bytes,
+  # since SETTINGS values are 32-bit (RFC 9113 6.5.1).
+  @max_dynamic_table_size_updates_size 2 * 6
+
   defp assert_header_block_within_max_size(conn, size) do
     case conn.client_settings.max_header_list_size do
       :infinity ->
         conn
 
-      max_size when size > max_size ->
-        debug_data = "header block exceeds SETTINGS_MAX_HEADER_LIST_SIZE of #{max_size} bytes"
-        send_connection_error!(conn, :protocol_error, debug_data)
+      max_size ->
+        max_block_size = div(max_size * 30, 8) + @max_dynamic_table_size_updates_size
 
-      _max_size ->
-        conn
+        if size > max_block_size do
+          debug_data =
+            "header block fragments exceed #{max_block_size} bytes, the bound for " <>
+              "SETTINGS_MAX_HEADER_LIST_SIZE of #{max_size} bytes"
+
+          send_connection_error!(conn, :protocol_error, debug_data)
+        else
+          conn
+        end
     end
   end
 

test/mint/http2/conn_test.exs

@@ -1052,10 +1052,11 @@ defmodule Mint.HTTP2Test do
       assert_recv_frames [headers(stream_id: stream_id)]
 
       # Each CONTINUATION is individually under the limit, but together they
-      # accumulate past the advertised SETTINGS_MAX_HEADER_LIST_SIZE. The client
-      # must refuse to buffer the header block without bound rather than growing
-      # `headers_being_processed` until it runs out of memory.
-      chunk = :binary.copy(<<0>>, 400)
+      # accumulate past the upper bound on the encoded size of a header list
+      # within the advertised SETTINGS_MAX_HEADER_LIST_SIZE (1_000 * 30 / 8 + 12
+      # bytes). The client must refuse to buffer the header block without bound
+      # rather than growing `headers_being_processed` until it runs out of memory.
+      chunk = :binary.copy(<<0>>, 1_300)
 
       assert {:error, %HTTP2{} = conn, error, []} =
                stream_frames(conn, [
@@ -1085,6 +1086,74 @@ defmodule Mint.HTTP2Test do
       refute HTTP2.open?(conn)
     end
 
+    @tag connect_options: [client_settings: [max_header_list_size: 1_000]]
+    test "header block fragments buffered up to the encoded size bound are accepted",
+         %{conn: conn} do
+      {conn, _ref} = open_request(conn)
+
+      assert_recv_frames [headers(stream_id: stream_id)]
+
+      # 1_000 * 30 / 8 + 12 = 3_762 bytes.
+      assert {:ok, %HTTP2{} = conn, []} =
+               stream_frames(conn, [
+                 headers(
+                   stream_id: stream_id,
+                   hbf: :binary.copy(<<0>>, 1_001),
+                   flags: set_flags(:headers, [])
+                 ),
+                 continuation(
+                   stream_id: stream_id,
+                   hbf: :binary.copy(<<0>>, 2_761),
+                   flags: set_flags(:continuation, [])
+                 )
+               ])
+
+      assert {^stream_id, _hbf, _callback, 3_762} = conn.headers_being_processed
+      assert HTTP2.open?(conn)
+
+      assert {:error, %HTTP2{} = conn, error, []} =
+               stream_frames(conn, [
+                 continuation(
+                   stream_id: stream_id,
+                   hbf: <<0>>,
+                   flags: set_flags(:continuation, [])
+                 )
+               ])
+
+      assert_http2_error error, {:protocol_error, debug_data}
+      assert debug_data =~ "fragments exceed 3762 bytes"
+      refute HTTP2.open?(conn)
+    end
+
+    @tag connect_options: [client_settings: [max_header_list_size: 1_000]]
+    test "the fragment that ends a header block is not counted against the buffered size",
+         %{conn: conn} do
+      {conn, ref} = open_request(conn)
+
+      assert_recv_frames [headers(stream_id: stream_id)]
+
+      # 4_000 dynamic table size updates to 0 followed by the indexed :status 200
+      # field are 4_001 bytes that decode to a 42-byte header list.
+      hbf = :binary.copy(<<0x20>>, 4_000) <> <<0x88>>
+      {hbf1, hbf2} = :erlang.split_binary(hbf, 1_000)
+
+      assert {:ok, %HTTP2{} = conn, [{:status, ^ref, 200}, {:headers, ^ref, []}, {:done, ^ref}]} =
+               stream_frames(conn, [
+                 headers(
+                   stream_id: stream_id,
+                   hbf: hbf1,
+                   flags: set_flags(:headers, [:end_stream])
+                 ),
+                 continuation(
+                   stream_id: stream_id,
+                   hbf: hbf2,
+                   flags: set_flags(:continuation, [:end_headers])
+                 )
+               ])
+
+      assert HTTP2.open?(conn)
+    end
+
     # Regression for GitHub Security Advisory GHSA-8pf6-g464-h6h9.
     test "empty CONTINUATION frames do not grow the header block accumulator", %{conn: conn} do
       {conn, _ref} = open_request(conn)
@@ -1251,6 +1320,40 @@ defmodule Mint.HTTP2Test do
       assert HTTP2.open?(conn)
     end
 
+    # RFC 7541 5.2: Huffman coding can make a string longer than its octets, so an
+    # encoded block can be larger than the header list it decodes to.
+    @tag connect_options: [client_settings: [max_header_list_size: 1_000]]
+    test "a Huffman-coded header list under max_header_list_size is accepted when split " <>
+           "over CONTINUATION frames",
+         %{conn: conn} do
+      {conn, ref} = open_request(conn)
+
+      assert_recv_frames [headers(stream_id: stream_id)]
+
+      # A 475-byte header list that encodes to 1_307 bytes.
+      value = :binary.copy(<<255>>, 400)
+      hbf = huffman_encode_headers([{":status", "200"}, {"x", value}])
+      assert byte_size(hbf) == 1_307
+      {hbf1, hbf2} = :erlang.split_binary(hbf, 1_001)
+
+      assert {:ok, %HTTP2{} = conn, responses} =
+               stream_frames(conn, [
+                 headers(
+                   stream_id: stream_id,
+                   hbf: hbf1,
+                   flags: set_flags(:headers, [:end_stream])
+                 ),
+                 continuation(
+                   stream_id: stream_id,
+                   hbf: hbf2,
+                   flags: set_flags(:continuation, [:end_headers])
+                 )
+               ])
+
+      assert [{:status, ^ref, 200}, {:headers, ^ref, [{"x", ^value}]}, {:done, ^ref}] = responses
+      assert HTTP2.open?(conn)
+    end
+
     @tag connect_options: [client_settings: [max_header_list_size: 1_000]]
     test "a decoded trailer list past max_header_list_size is a stream error", %{conn: conn} do
       {conn, ref} = open_request(conn)
@@ -1303,7 +1406,7 @@ defmodule Mint.HTTP2Test do
 
       assert_recv_frames [headers(stream_id: stream_id)]
 
-      oversized_hbf = :binary.copy(<<0>>, 2_000)
+      oversized_hbf = :binary.copy(<<0>>, 3_763)
 
       assert {:error, %HTTP2{} = conn, error, []} =
                stream_frames(conn, [
@@ -4766,6 +4869,13 @@ defmodule Mint.HTTP2Test do
     hbf
   end
 
+  # Encodes without touching the dynamic table, so the server's HPAX context stays in sync.
+  defp huffman_encode_headers(headers) do
+    table = HPAX.new(4096, huffman_encoding: :always)
+    {hbf, _table} = HPAX.encode(:no_store, headers, table)
+    IO.iodata_to_binary(hbf)
+  end
+
   defp server_decode_headers(hbf) do
     server = Process.get(@server_pdict_key)
     {server, headers} = TestServer.decode_headers(server, hbf)