From a38a401da27217474148661f7e137bb573de38e1 Mon Sep 17 00:00:00 2001 From: Neil Berkman Date: Sat, 18 Oct 2025 12:07:33 -0700 Subject: [PATCH 1/2] Add test case for issue #265 (ALPN + large body bug) Reproduces the bug where POST requests with bodies >64KB fail when using protocols: [:http1, :http2] due to HTTP/2 flow control not being implemented in the HTTP/1 pool. The failing test is marked with @tag :skip for future work. Two passing control tests confirm HTTP/2-only and HTTP/1-only configurations work correctly. --- test/finch/alpn_integration_test.exs | 121 +++++++++++++++++++++++++++ test/support/alpn_server.ex | 60 +++++++++++++ 2 files changed, 181 insertions(+) create mode 100644 test/finch/alpn_integration_test.exs create mode 100644 test/support/alpn_server.ex diff --git a/test/finch/alpn_integration_test.exs b/test/finch/alpn_integration_test.exs new file mode 100644 index 0000000..49e67fb --- /dev/null +++ b/test/finch/alpn_integration_test.exs @@ -0,0 +1,121 @@ +defmodule Finch.ALPNIntegrationTest do + use ExUnit.Case, async: false + + @moduletag :capture_log + + setup_all do + {:ok, listen_socket} = :ssl.listen(0, mode: :binary) + {:ok, {_address, port}} = :ssl.sockname(listen_socket) + :ssl.close(listen_socket) + + {:ok, _} = Finch.ALPNServer.start(port) + + {:ok, url: "https://localhost:#{port}"} + end + + # This test reproduces issue #265 where sending POST requests with bodies larger than 64KB + # fails when using protocols: [:http1, :http2] due to HTTP/2 window size constraints. + # The HTTP/1 pool doesn't implement HTTP/2 flow control, causing the request to exceed + # the HTTP/2 window size (65535 bytes). + @tag :skip + test "POST request with body larger than 64KB using ALPN negotiation", %{url: url} do + # Start Finch with ALPN negotiation (both HTTP/1 and HTTP/2) + start_supervised!( + {Finch, + name: ALPNFinch, + pools: %{ + default: [ + protocols: [:http1, :http2], + conn_opts: [ + transport_opts: [ + verify: :verify_none + ] + ] + ] + }} + ) + + # Create a body larger than 64KB (65538 bytes as per issue) + large_body = :crypto.strong_rand_bytes(65_538) + + # This should fail with {:exceeds_window_size, :request, 65535} + # when the connection upgrades to HTTP/2 via ALPN + result = + Finch.build(:post, "#{url}/echo", [], large_body) + |> Finch.request(ALPNFinch) + + # Currently this fails with the window size error + # Once fixed, this should succeed + case result do + {:ok, response} -> + assert response.status == 200 + body = Jason.decode!(response.body) + assert body["received_bytes"] == 65_538 + + {:error, %Mint.HTTPError{reason: {:exceeds_window_size, :request, 65535}}} -> + flunk(""" + Request failed with window size error - this is the bug we're trying to fix. + The HTTP/1 pool doesn't implement HTTP/2 flow control when connections upgrade via ALPN. + """) + + {:error, error} -> + flunk("Unexpected error: #{inspect(error)}") + end + end + + # Test that confirms HTTP/2-only works correctly with large bodies + test "POST request with body larger than 64KB using HTTP/2 only (should work)", %{url: url} do + start_supervised!( + {Finch, + name: HTTP2Finch, + pools: %{ + default: [ + protocols: [:http2], + conn_opts: [ + transport_opts: [ + verify: :verify_none + ] + ] + ] + }} + ) + + large_body = :crypto.strong_rand_bytes(65_538) + + {:ok, response} = + Finch.build(:post, "#{url}/echo", [], large_body) + |> Finch.request(HTTP2Finch) + + assert response.status == 200 + body = Jason.decode!(response.body) + assert body["received_bytes"] == 65_538 + end + + # Test that confirms HTTP/1-only works correctly with large bodies + test "POST request with body larger than 64KB using HTTP/1 only (should work)", %{url: url} do + start_supervised!( + {Finch, + name: HTTP1Finch, + pools: %{ + default: [ + protocols: [:http1], + conn_opts: [ + transport_opts: [ + verify: :verify_none + ] + ] + ] + }} + ) + + large_body = :crypto.strong_rand_bytes(65_538) + + {:ok, response} = + Finch.build(:post, "#{url}/echo", [], large_body) + |> Finch.request(HTTP1Finch) + + assert response.status == 200 + body = Jason.decode!(response.body) + assert body["received_bytes"] == 65_538 + end +end diff --git a/test/support/alpn_server.ex b/test/support/alpn_server.ex new file mode 100644 index 0000000..673bc12 --- /dev/null +++ b/test/support/alpn_server.ex @@ -0,0 +1,60 @@ +defmodule Finch.ALPNServer do + @moduledoc false + # A test server that supports ALPN negotiation between HTTP/1 and HTTP/2 + + @fixtures_dir Path.expand("../fixtures", __DIR__) + + def child_spec(opts) do + Plug.Cowboy.child_spec( + scheme: :https, + plug: Finch.ALPNServer.PlugRouter, + options: [ + port: Keyword.fetch!(opts, :port), + cipher_suite: :strong, + certfile: Path.join([@fixtures_dir, "selfsigned.pem"]), + keyfile: Path.join([@fixtures_dir, "selfsigned_key.pem"]), + # Enable ALPN negotiation between HTTP/2 and HTTP/1.1 + alpn_preferred_protocols: ["h2", "http/1.1"], + otp_app: :finch, + protocol_options: [ + idle_timeout: 3_000, + inactivity_timeout: 5_000, + max_keepalive: 1_000, + request_timeout: 10_000, + shutdown_timeout: 10_000 + ] + ] + ) + end + + def start(port) do + Supervisor.start_link([child_spec(port: port)], strategy: :one_for_one) + end +end + +defmodule Finch.ALPNServer.PlugRouter do + @moduledoc false + + use Plug.Router + + plug(:match) + plug(:dispatch) + + get "/" do + conn + |> send_resp(200, "Hello world!") + |> halt() + end + + post "/echo" do + {:ok, body, conn} = read_body(conn) + body_size = byte_size(body) + + response = Jason.encode!(%{received_bytes: body_size}) + + conn + |> put_resp_content_type("application/json") + |> send_resp(200, response) + |> halt() + end +end From 75d06a8baed9bcffc9ba9cd27b4d290bca55edbc Mon Sep 17 00:00:00 2001 From: Neil Berkman Date: Tue, 21 Oct 2025 01:32:41 -0700 Subject: [PATCH 2/2] Fix Credo readability warning for large number formatting Change 65535 to 65_535 to comply with Credo's readability check that numbers larger than 9999 should use underscore separators. --- test/finch/alpn_integration_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/finch/alpn_integration_test.exs b/test/finch/alpn_integration_test.exs index 49e67fb..2318ba0 100644 --- a/test/finch/alpn_integration_test.exs +++ b/test/finch/alpn_integration_test.exs @@ -52,7 +52,7 @@ defmodule Finch.ALPNIntegrationTest do body = Jason.decode!(response.body) assert body["received_bytes"] == 65_538 - {:error, %Mint.HTTPError{reason: {:exceeds_window_size, :request, 65535}}} -> + {:error, %Mint.HTTPError{reason: {:exceeds_window_size, :request, 65_535}}} -> flunk(""" Request failed with window size error - this is the bug we're trying to fix. The HTTP/1 pool doesn't implement HTTP/2 flow control when connections upgrade via ALPN.