From 701c5dac3655d19bbe22cb08747629299f760b2b Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Thu, 24 Sep 2026 09:52:21 +0200 Subject: [PATCH] Cover the HTTP/3 side of the header fix, and widen a test's margins normalize_headers/1 is shared by the HTTP/2 and HTTP/3 request paths, so stripping the connection-specific headers of RFC 9113 8.2.2 fixed HTTP/3 too, where RFC 9114 4.2 bans the same fields. Only HTTP/2 was covered, so narrowing that filter would have broken HTTP/3 quietly. The HTTP/3 test server reports the headers it received on a new /headers route, and its route list gains the /reset entry it was missing. The checkout-idle tests probed within 100ms of a 300ms deadline, which is thin for the FreeBSD and macOS runners. The timer is 1s now and the probe sits half a second from either side of it. The assertions are unchanged. --- test/hackney_h3_test_server.erl | 12 +++ ...hackney_http3_connection_headers_tests.erl | 75 +++++++++++++++++++ test/hackney_pool_checkout_idle_tests.erl | 16 ++-- 3 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 test/hackney_http3_connection_headers_tests.erl diff --git a/test/hackney_h3_test_server.erl b/test/hackney_h3_test_server.erl index 8c4fb4f8..732bd5b7 100644 --- a/test/hackney_h3_test_server.erl +++ b/test/hackney_h3_test_server.erl @@ -15,6 +15,11 @@ %%% GET /large 200, a 64 KiB body sent in 16 DATA frames %%% GET /redirect/N 302 to /redirect/N-1; /redirect/0 is 200 %%% GET /status/N status N; a 3xx carries location: / +%%% GET /headers 200 text/plain, the request headers it received, +%%% one "name: value" per line, sorted +%%% GET /reset 200 and part of a body, then resets the stream +%%% when the process registered as +%%% hackney_h3_test_reset sends `reset' %%% POST any path 200, echoes the request body %%% anything else 404 -module(hackney_h3_test_server). @@ -145,6 +150,13 @@ handle(Conn, StreamId, <<"GET">>, <<"/status/", N/binary>>, _Headers) -> false -> [] end, respond(Conn, StreamId, Status, Headers, <<>>); +handle(Conn, StreamId, <<"GET">>, <<"/headers">>, Headers) -> + %% Report the headers that reached the server, so a test can assert on + %% what hackney put on the wire. + Lines = [[Name, <<": ">>, Value, <<"\n">>] + || {Name, Value} <- lists:sort(Headers)], + respond(Conn, StreamId, 200, [{<<"content-type">>, <<"text/plain">>}], + iolist_to_binary(Lines)); handle(Conn, StreamId, <<"GET">>, <<"/reset">>, _Headers) -> quic_h3:send_response(Conn, StreamId, 200, [{<<"content-type">>, <<"text/plain">>}]), quic_h3:send_data(Conn, StreamId, <<"part">>, false), diff --git a/test/hackney_http3_connection_headers_tests.erl b/test/hackney_http3_connection_headers_tests.erl new file mode 100644 index 00000000..f5b1e157 --- /dev/null +++ b/test/hackney_http3_connection_headers_tests.erl @@ -0,0 +1,75 @@ +%%% -*- erlang -*- +%%% +%%% This file is part of hackney released under the Apache 2 license. +%%% See the NOTICE for more information. +%%% +%%% Copyright (c) 2026 Benoit Chesneau +%%% +%%% @doc Connection-specific headers are dropped from HTTP/3 requests. +%%% +%%% RFC 9114 4.2 bans them, as RFC 9113 8.2.2 does for HTTP/2, and +%%% hackney_conn:normalize_headers/1 is shared by both protocols. The +%%% HTTP/2 side is covered by hackney_http2_connection_headers_tests; these +%%% pin the HTTP/3 side, so narrowing that filter to HTTP/2 cannot pass +%%% unnoticed. The server reports the headers it received on /headers. +-module(hackney_http3_connection_headers_tests). + +-include_lib("eunit/include/eunit.hrl"). + +connection_headers_test_() -> + {setup, + fun hackney_h3_test_server:start/0, + fun hackney_h3_test_server:stop/1, + fun(Server) -> + [{"a request carrying Connection: keep-alive still succeeds over h3", + {timeout, 30, fun() -> keep_alive_header_is_dropped(Server) end}}, + {"every connection-specific header is dropped, whatever the casing", + {timeout, 30, fun() -> all_connection_headers_are_dropped(Server) end}}, + {"ordinary headers are still delivered", + {timeout, 30, fun() -> ordinary_headers_survive(Server) end}}] + end}. + +keep_alive_header_is_dropped(Server) -> + Received = request(Server, [{<<"Connection">>, <<"keep-alive">>}]), + ?assertEqual([], connection_specific(Received)). + +all_connection_headers_are_dropped(Server) -> + Received = request(Server, [{<<"Connection">>, <<"keep-alive">>}, + {<<"keep-alive">>, <<"timeout=5">>}, + {<<"Proxy-Connection">>, <<"keep-alive">>}, + {<<"Transfer-Encoding">>, <<"chunked">>}, + {<<"UPGRADE">>, <<"websocket">>}]), + ?assertEqual([], connection_specific(Received)). + +ordinary_headers_survive(Server) -> + Received = request(Server, [{<<"Connection">>, <<"keep-alive">>}, + {<<"X-Api-Version">>, <<"4">>}, + {<<"Accept">>, <<"application/json">>}]), + ?assertEqual([], connection_specific(Received)), + ?assertEqual(<<"4">>, proplists:get_value(<<"x-api-version">>, Received)), + ?assertEqual(<<"application/json">>, + proplists:get_value(<<"accept">>, Received)). + +%%==================================================================== +%% Helpers +%%==================================================================== + +%% Ask the server which headers reached it, as name/value pairs. +request(Server, Headers) -> + URL = hackney_h3_test_server:url(Server, <<"/headers">>), + Opts = [{with_body, true} | hackney_h3_test_server:hackney_opts()], + {ok, 200, _RespHeaders, Body} = hackney:request(get, URL, Headers, <<>>, Opts), + parse_headers(Body). + +parse_headers(Body) -> + [split_header(Line) || Line <- binary:split(Body, <<"\n">>, [global]), + Line =/= <<>>]. + +split_header(Line) -> + [Name, Value] = binary:split(Line, <<": ">>), + {Name, Value}. + +connection_specific(Headers) -> + Banned = [<<"connection">>, <<"keep-alive">>, <<"proxy-connection">>, + <<"transfer-encoding">>, <<"upgrade">>], + [Name || {Name, _} <- Headers, lists:member(Name, Banned)]. diff --git a/test/hackney_pool_checkout_idle_tests.erl b/test/hackney_pool_checkout_idle_tests.erl index eb1c2b57..1506a06f 100644 --- a/test/hackney_pool_checkout_idle_tests.erl +++ b/test/hackney_pool_checkout_idle_tests.erl @@ -16,7 +16,7 @@ -include_lib("eunit/include/eunit.hrl"). -define(BODY, <<"{\"ok\":true}">>). --define(IDLE_MS, 300). +-define(IDLE_MS, 1000). checkout_idle_test_() -> {setup, @@ -37,11 +37,13 @@ checkout_disarms_idle_timer() -> try {ok, 200, _} = hackney_conn:request(ConnPid, <<"GET">>, <<"/">>, [], <<>>), {ok, ?BODY} = hackney_conn:body(ConnPid), - %% Idle in connected, timer armed for ?IDLE_MS. Sit on most of it. - timer:sleep(?IDLE_MS - 100), + %% Idle in connected, timer armed for ?IDLE_MS. Probe halfway through: + %% far enough in to matter, far enough from the deadline that a slow + %% runner cannot close the conn before the probe. + timer:sleep(?IDLE_MS div 2), ?assertEqual({ok, connected}, hackney_conn:is_ready(ConnPid)), %% Past the original deadline. Before the fix the conn had closed here. - timer:sleep(200), + timer:sleep(?IDLE_MS), ?assertMatch({connected, _}, sys:get_state(ConnPid)) after catch hackney_conn:close(ConnPid), @@ -55,9 +57,9 @@ request_after_checkout_succeeds() -> try {ok, 200, _} = hackney_conn:request(ConnPid, <<"GET">>, <<"/">>, [], <<>>), {ok, ?BODY} = hackney_conn:body(ConnPid), - timer:sleep(?IDLE_MS - 100), + timer:sleep(?IDLE_MS div 2), ?assertEqual({ok, connected}, hackney_conn:is_ready(ConnPid)), - timer:sleep(200), + timer:sleep(?IDLE_MS), %% {error, invalid_state} before the fix. ?assertMatch({ok, 200, _}, hackney_conn:request(ConnPid, <<"GET">>, <<"/">>, [], <<>>)), @@ -79,7 +81,7 @@ idle_timer_rearmed_after_response() -> {ok, 200, _} = hackney_conn:request(ConnPid, <<"GET">>, <<"/">>, [], <<>>), {ok, ?BODY} = hackney_conn:body(ConnPid), %% Now idle again with the timer re-armed; it must still fire. - timer:sleep(?IDLE_MS + 200), + timer:sleep(?IDLE_MS + 500), ?assertNotMatch({connected, _}, sys:get_state(ConnPid)) after catch hackney_conn:close(ConnPid),