fix: strip connection-specific headers from HTTP/2 requests

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).

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"]).