From 2ccaebb3eb24b118f4aa07ed9b4e8f96eb025e2f Mon Sep 17 00:00:00 2001 From: Lennart Schoch Date: Fri, 28 Aug 2026 12:15:36 +0100 Subject: [PATCH] Strip connection-specific headers from HTTP/2 requests RFC 9113 8.2.2 bans connection, keep-alive, proxy-connection, transfer-encoding and upgrade, and the h2 layer refuses to send a block containing one, so the request fails with protocol_error before it reaches the socket. They are legal in HTTP/1.1 and the caller cannot know which protocol ALPN picked, so drop them next to Host rather than failing requests that are valid for hackney's own API. --- src/hackney_conn.erl | 22 +++- ...hackney_http2_connection_headers_tests.erl | 122 ++++++++++++++++++ 2 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 test/hackney_http2_connection_headers_tests.erl diff --git a/src/hackney_conn.erl b/src/hackney_conn.erl index 73ec3243..36408837 100644 --- a/src/hackney_conn.erl +++ b/src/hackney_conn.erl @@ -3368,15 +3368,31 @@ to_binary(V) when is_atom(V) -> atom_to_binary(V, utf8). %% @private Normalize headers to binary key-value pairs for HTTP/2 %% Also filters out Host header since :authority pseudo-header is used instead. %% Having both Host and :authority causes protocol_error on strict servers (e.g. Google). +%% The connection-specific headers go too: RFC 9113 8.2.2 forbids them, so the h2 +%% layer refuses to send a block containing one and the request fails before it +%% reaches the socket. They are perfectly legal in HTTP/1.1, and a caller cannot +%% know which protocol ALPN picked, so hackney drops them here rather than failing +%% requests that are valid for its own API. normalize_headers(Headers) -> lists:filtermap(fun({K, V}) -> KeyLower = hackney_bstr:to_lower(to_binary(K)), - case KeyLower of - <<"host">> -> false; %% Skip Host header - use :authority instead - _ -> {true, {KeyLower, to_binary(V)}} + case is_connection_specific(KeyLower) of + true -> false; + false -> {true, {KeyLower, to_binary(V)}} end end, Headers). +%% @private Host is carried by :authority; the rest are the connection-specific +%% header fields of RFC 9113 8.2.2. TE is left alone: it is allowed with the +%% single value "trailers", and the h2 layer validates that itself. +is_connection_specific(<<"host">>) -> true; +is_connection_specific(<<"connection">>) -> true; +is_connection_specific(<<"keep-alive">>) -> true; +is_connection_specific(<<"proxy-connection">>) -> true; +is_connection_specific(<<"transfer-encoding">>) -> true; +is_connection_specific(<<"upgrade">>) -> true; +is_connection_specific(_) -> false. + %% @private Handle a {h2, Conn, Event} owner message from the h2 library. handle_h2_event({informational, _StreamId, _Status, _Headers}, Data) -> %% 1xx interim responses: ignore for now (same as previous HPACK path). diff --git a/test/hackney_http2_connection_headers_tests.erl b/test/hackney_http2_connection_headers_tests.erl new file mode 100644 index 00000000..216d80fe --- /dev/null +++ b/test/hackney_http2_connection_headers_tests.erl @@ -0,0 +1,122 @@ +%%% -*- erlang -*- +%%% +%%% This file is part of hackney released under the Apache 2 license. +%%% See the NOTICE for more information. +%%% +%%% Connection-specific request headers must not reach an HTTP/2 request. +%%% +%%% RFC 9113 8.2.2 forbids connection, keep-alive, proxy-connection, +%%% transfer-encoding and upgrade in HTTP/2. The h2 layer validates outbound +%%% header blocks and replies {error, protocol_error} when one is present, so a +%%% caller that sets any of them - which is legal in HTTP/1.1, and common in +%%% clients written before ALPN could pick h2 - had every request fail. +-module(hackney_http2_connection_headers_tests). + +-include_lib("eunit/include/eunit.hrl"). + +-define(BODY, <<"ok">>). + +connection_headers_test_() -> + {setup, + fun() -> + _ = application:ensure_all_started(hackney), + _ = application:ensure_all_started(h2), + ok + end, + fun(_) -> ok end, + [{"a request carrying Connection: keep-alive still succeeds over h2", + {timeout, 30, fun keep_alive_header_is_dropped/0}}, + {"every connection-specific header is dropped, whatever the casing", + {timeout, 30, fun all_connection_headers_are_dropped/0}}, + {"ordinary headers are still delivered", + {timeout, 30, fun ordinary_headers_survive/0}}]}. + +keep_alive_header_is_dropped() -> + {Server, Port, Seen} = start_server(), + try + ?assertMatch({ok, 200, _, ?BODY}, + request(Port, [{<<"Connection">>, <<"keep-alive">>}])), + ?assertEqual([], connection_specific(received_headers(Seen))) + after + stop_server(Server) + end. + +all_connection_headers_are_dropped() -> + {Server, Port, Seen} = start_server(), + Headers = [{<<"Connection">>, <<"keep-alive">>}, + {<<"keep-alive">>, <<"timeout=5">>}, + {<<"Proxy-Connection">>, <<"keep-alive">>}, + {<<"Transfer-Encoding">>, <<"chunked">>}, + {<<"UPGRADE">>, <<"h2c">>}], + try + ?assertMatch({ok, 200, _, ?BODY}, request(Port, Headers)), + ?assertEqual([], connection_specific(received_headers(Seen))) + after + stop_server(Server) + end. + +%% The filter must not be over-eager: anything not on the list still goes. +ordinary_headers_survive() -> + {Server, Port, Seen} = start_server(), + try + ?assertMatch({ok, 200, _, ?BODY}, + request(Port, [{<<"Connection">>, <<"keep-alive">>}, + {<<"X-Api-Version">>, <<"4">>}, + {<<"Accept">>, <<"application/json">>}])), + Received = received_headers(Seen), + ?assertEqual(<<"4">>, proplists:get_value(<<"x-api-version">>, Received)), + ?assertEqual(<<"application/json">>, + proplists:get_value(<<"accept">>, Received)) + after + stop_server(Server) + end. + +request(Port, Headers) -> + URL = iolist_to_binary([<<"https://localhost:">>, integer_to_list(Port), <<"/">>]), + hackney:request(get, URL, Headers, <<>>, + [{protocols, [http2]}, + {pool, false}, + {with_body, true}, + {recv_timeout, 5000}, + {ssl_options, [{insecure, true}, {verify, verify_none}]}]). + +connection_specific(Headers) -> + Banned = [<<"connection">>, <<"keep-alive">>, <<"proxy-connection">>, + <<"transfer-encoding">>, <<"upgrade">>], + [N || {N, _} <- Headers, lists:member(N, Banned)]. + +received_headers(Seen) -> + receive + {headers, H} -> H + after 5000 -> + exit({no_request_reached_server, Seen}) + end. + +%%==================================================================== +%% Minimal HTTP/2 server that reports the request headers it saw. +%%==================================================================== + +start_server() -> + Self = self(), + Handler = fun(Conn, Sid, _Method, _Path, Headers) -> + Self ! {headers, Headers}, + ok = h2:send_response(Conn, Sid, 200, + [{<<"content-type">>, <<"text/plain">>}]), + ok = h2:send_data(Conn, Sid, ?BODY, true) + end, + Certs = cert_dir(), + {ok, Server} = h2:start_server(0, #{ + cert => filename:join(Certs, "server.pem"), + key => filename:join(Certs, "server.key"), + handler => Handler + }), + {Server, h2:server_port(Server), Self}. + +stop_server(Server) -> + catch h2:stop_server(Server), + ok. + +cert_dir() -> + BeamDir = filename:dirname(code:which(?MODULE)), + Root = filename:join([BeamDir, "..", "..", "..", "..", ".."]), + filename:join([filename:absname(Root), "test", "certs"]).