Skip to content

Fix HTTP/2-on-HTTP/1 mislabel on resumed TLS sessions and the recv_status spin #889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
benoitc merged 4 commits into from
Jun 18, 2026
Merged

Fix HTTP/2-on-HTTP/1 mislabel on resumed TLS sessions and the recv_status spin #889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f703a47
Fix HTTP/2-on-HTTP/1 mislabel on resumed TLS sessions and the recv_st…
benoitc Jun 18, 2026
ffb5a36
Add 4.4.5 changelog entry
benoitc Jun 18, 2026
a1fbeea
Scope ALPN memo to the resumable ticket source
benoitc Jun 18, 2026
072c5da
Only automatic tickets make a conn ALPN-memo-eligible
benoitc Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
# NEWS

4.4.5 - 2026-06-18
------------------

### Fixed

- HTTPS: a connection reused over a resumed TLS 1.3 session is no longer
mislabeled as HTTP/1 when it negotiated HTTP/2. `ssl:negotiated_protocol/1`
reports nothing on a resumed session, so hackney now remembers the protocol
learned on the full handshake (per host and advertised ALPN) and offers
resumption only once that protocol is known, resolving a resumed session
against that snapshot. Reused h2 connections take the h2 path instead of
feeding h2 frames to the HTTP/1 parser.
- HTTP/1.1: a response that cannot begin an HTTP/1 status line (for example an
HTTP/2 frame on a mislabeled connection) now fails fast with
`{error, {bad_response, not_http}}` instead of spinning the CPU in the
status-line parser.
- Connection pooling: `Connection: close` responses are no longer returned to
the pool on the sync body path; checkin only pools connections proven
keep-alive and socket-ready (unknown defaults to close); and a closed pooled
entry is discarded at checkout instead of being redialed inside the pool
process (#888).

4.4.4 - 2026-06-17
------------------

Expand Down
119 changes: 87 additions & 32 deletions src/hackney_conn.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -898,10 +898,18 @@ connected({call, From}, {upgrade_to_ssl, SslOpts, UpgradeOpts}, #conn_data{socke
end,
hackney_util:merge_opts(MergedSslOpts, AlpnOpts)
end,
case ssl:connect(Socket, FinalSslOpts) of
%% Gate TLS resumption on the ALPN memo and snapshot the cached protocol, so a
%% resumed session (where ssl reports no ALPN) resolves against this snapshot.
%% Resumable: only the resumption-eligible config (session_tickets enabled)
%% updates the memo, so a custom-ssl_options handshake cannot poison it.
AlpnProtos = alpn_advertised(FinalSslOpts),
Resumable = hackney_ssl:auto_tickets(FinalSslOpts),
Cached = hackney_ssl:recall_alpn(Host, AlpnProtos),
GatedSslOpts = gate_resumption(FinalSslOpts, Cached),
case ssl:connect(Socket, GatedSslOpts) of
{ok, SslSocket} ->
%% Detect negotiated protocol
Protocol = hackney_ssl:get_negotiated_protocol(SslSocket),
%% Detect negotiated protocol, carrying ALPN across resumption
Protocol = hackney_ssl:negotiated_protocol(SslSocket, Host, AlpnProtos, Cached, Resumable),
%% Update connection to use SSL
NewData = Data#conn_data{
transport = hackney_ssl,
Expand Down Expand Up @@ -2183,21 +2191,22 @@ recv_status_and_headers_loop(Data) ->
recv_status(#conn_data{parser = Parser, buffer = Buffer} = Data) ->
case hackney_http:execute(Parser, Buffer) of
{more, NewParser} ->
%% Check if parser has data in its internal buffer (e.g., after skipping 1XX response)
%% If so, continue parsing; otherwise read from socket
ParserBuffer = hackney_http:get(NewParser, buffer),
case ParserBuffer of
<<>> ->
%% Parser buffer empty - need to read from socket
%% The parser needs more bytes to complete the status line. It already
%% consumed what it could, so re-feeding its own buffer makes no
%% progress (that was an infinite spin); always read from the socket.
%% But a valid HTTP/1 status line starts with "HTTP/": if what we have
%% can never be that (e.g. an HTTP/2 frame on a mislabeled connection),
%% fail fast instead of reading until recv_timeout.
case maybe_http_status_start(hackney_http:get(NewParser, buffer)) of
false ->
{error, {bad_response, not_http}};
true ->
case recv_data(Data) of
{ok, RecvData} ->
recv_status(Data#conn_data{parser = NewParser, buffer = RecvData});
{error, Reason} ->
{error, Reason}
end;
_ ->
%% Parser has data in buffer - continue parsing without reading
recv_status(Data#conn_data{parser = NewParser, buffer = <<>>})
end
end;
{response, Version, Status, Reason, NewParser} ->
recv_headers(Data#conn_data{
Expand All @@ -2211,6 +2220,26 @@ recv_status(#conn_data{parser = Parser, buffer = Buffer} = Data) ->
{error, Reason}
end.

%% @private Whether the buffered bytes can still be the start of an HTTP/1 status
%% line. Leading blank lines are tolerated (RFC 7230 3.5). True while the buffer is
%% empty or shares a common prefix with "HTTP/" up to the shorter length, so both a
%% still-accumulating version ("HTT") and a longer partial line ("HTTP/1.1 2") pass;
%% false once it diverges (e.g. an HTTP/2 frame), so a protocol mismatch fails fast.
maybe_http_status_start(Buffer) ->
case strip_leading_eol(Buffer) of
<<>> ->
true;
Bin ->
Prefix = <<"HTTP/">>,
N = min(byte_size(Bin), byte_size(Prefix)),
binary:part(Bin, 0, N) =:= binary:part(Prefix, 0, N)
end.

%% @private Drop leading CR/LF bytes (lenient about blank lines before the status line).
strip_leading_eol(<<"\r", Rest/binary>>) -> strip_leading_eol(Rest);
strip_leading_eol(<<"\n", Rest/binary>>) -> strip_leading_eol(Rest);
strip_leading_eol(Bin) -> Bin.

recv_headers(#conn_data{parser = Parser} = Data, Headers) ->
case hackney_http:execute(Parser) of
{more, NewParser} ->
Expand Down Expand Up @@ -2687,7 +2716,7 @@ do_tcp_connect(From, Data) ->
} = Data,
%% Filter out hackney-specific options that are not valid for transport
TransportOpts = proplists:delete(protocols, ConnectOpts),
Opts = case Transport of
case Transport of
hackney_ssl ->
%% effective_opts is the single builder of ssl:connect options for
%% both default and custom ssl_options. It resolves SNI and ALPN
Expand All @@ -2697,27 +2726,53 @@ do_tcp_connect(From, Data) ->
undefined -> SslOpts0;
Protocols -> [{protocols, Protocols} | SslOpts0]
end,
FinalSslOpts = hackney_ssl:effective_opts(Host, SslOpts1, ConnectOpts),
TransportOpts ++ [{ssl_options, FinalSslOpts}];
_ -> TransportOpts
end,
case Transport:connect(Host, Port, Opts, Timeout) of
{ok, Socket} ->
Protocol = case Transport of
hackney_ssl -> hackney_ssl:get_negotiated_protocol(Socket);
_ -> http1
end,
case Protocol of
http2 ->
init_h2_connection(Socket, Data#conn_data{socket = Socket, protocol = http2}, From);
http1 ->
NewData = Data#conn_data{socket = Socket, protocol = http1},
{next_state, connected, NewData, [{reply, From, ok}]}
FinalSslOpts0 = hackney_ssl:effective_opts(Host, SslOpts1, ConnectOpts),
%% Gate TLS resumption on the ALPN memo and snapshot the cached
%% protocol now, so a resumed session (where ssl reports no ALPN) is
%% resolved against this snapshot rather than re-read from the memo.
%% Resumable: only the resumption-eligible config updates the memo.
AlpnProtos = alpn_advertised(FinalSslOpts0),
Resumable = hackney_ssl:auto_tickets(FinalSslOpts0),
Cached = hackney_ssl:recall_alpn(Host, AlpnProtos),
FinalSslOpts = gate_resumption(FinalSslOpts0, Cached),
Opts = TransportOpts ++ [{ssl_options, FinalSslOpts}],
case Transport:connect(Host, Port, Opts, Timeout) of
{ok, Socket} ->
case hackney_ssl:negotiated_protocol(Socket, Host, AlpnProtos, Cached, Resumable) of
http2 ->
init_h2_connection(Socket,
Data#conn_data{socket = Socket, protocol = http2}, From);
http1 ->
{next_state, connected,
Data#conn_data{socket = Socket, protocol = http1},
[{reply, From, ok}]}
end;
{error, Reason} ->
{stop_and_reply, normal, [{reply, From, {error, Reason}}]}
end;
{error, Reason} ->
{stop_and_reply, normal, [{reply, From, {error, Reason}}]}
_ ->
case Transport:connect(Host, Port, TransportOpts, Timeout) of
{ok, Socket} ->
{next_state, connected,
Data#conn_data{socket = Socket, protocol = http1},
[{reply, From, ok}]};
{error, Reason} ->
{stop_and_reply, normal, [{reply, From, {error, Reason}}]}
end
end.

%% @private The advertised ALPN protocol list (in offered order) from ssl opts,
%% or [] when none is offered. Used as part of the ALPN memo key.
alpn_advertised(SslOpts) ->
proplists:get_value(alpn_advertised_protocols, SslOpts, []).

%% @private Gate TLS resumption on the ALPN memo: keep `session_tickets' (offer
%% resumption) only once a full handshake has cached this host+ALPN's protocol.
%% A cold memo (`none') strips it so the handshake is full and reports ALPN, which
%% repopulates the memo. Keeps a resumed session from losing the protocol.
gate_resumption(SslOpts, none) -> proplists:delete(session_tickets, SslOpts);
gate_resumption(SslOpts, _Cached) -> SslOpts.

%% @private Initialize HTTP/2 connection via the h2 library.
%% The h2_connection process takes ownership of the socket and delivers
%% owner messages ({h2, Conn, Event}) to this gen_statem's mailbox.
Expand Down
122 changes: 115 additions & 7 deletions src/hackney_ssl.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,16 @@
-define(TLS_KEY_CACHE, hackney_tls_keys).
-define(TLS_KEY_CACHE_MAX, 512).

%% Per {Host, advertised-ALPN} memo of the protocol learned on a full handshake,
%% so a resumed TLS session (where ssl:negotiated_protocol/1 reports nothing) is
%% labeled correctly. See resolve_alpn/5 and the resumption gate in hackney_conn.
-define(ALPN_CACHE, hackney_alpn_protocols).
-define(ALPN_CACHE_MAX, 4096).

%% ALPN (Application-Layer Protocol Negotiation) for HTTP/2
-export([alpn_opts/1]).
-export([get_negotiated_protocol/1]).
-export([negotiated_protocol/5, resolve_alpn/6, recall_alpn/2, auto_tickets/1]).

%% @doc Atoms used to identify messages in {active, once | true} mode.
messages(_) -> {ssl, ssl_closed, ssl_error}.
Expand Down Expand Up @@ -152,15 +159,19 @@ tlsv13_allowed() ->
{ok, _} -> false
end.

%% @doc Create the TLS key memo table used by `effective_opts_and_key/3'.
%% Idempotent; called from hackney_sup:init/1.
%% @doc Create the TLS key memo table used by `effective_opts_and_key/3' and the
%% per-host ALPN memo used by `resolve_alpn/5'. Idempotent; called from
%% hackney_sup:init/1.
-spec init_key_cache() -> ok.
init_key_cache() ->
case ets:info(?TLS_KEY_CACHE) of
ok = ensure_table(?TLS_KEY_CACHE),
ok = ensure_table(?ALPN_CACHE),
ok.

ensure_table(Name) ->
case ets:info(Name) of
undefined ->
?TLS_KEY_CACHE = ets:new(?TLS_KEY_CACHE,
[set, public, named_table,
{read_concurrency, true}]),
Name = ets:new(Name, [set, public, named_table, {read_concurrency, true}]),
ok;
_ ->
ok
Expand Down Expand Up @@ -211,8 +222,13 @@ env_fingerprint() ->
%% lookup semantics) so option order does not change the key, while
%% conflicting duplicates such as `[{verify,A},{verify,B}]' and its
%% reverse still hash differently.
%%
%% `session_tickets' is excluded (like the HTTP/3 key excludes `session_ticket'):
%% it is identity-neutral (does not affect trust) and the handshake now varies it
%% per the ALPN resumption gate, so it must not change the pool bucket.
-spec options_key(list()) -> binary().
options_key(FinalSslOpts) ->
options_key(FinalSslOpts0) ->
FinalSslOpts = proplists:delete(session_tickets, FinalSslOpts0),
{Tuples, Rest} = lists:partition(
fun(T) -> is_tuple(T) andalso tuple_size(T) =:= 2 end,
FinalSslOpts),
Expand Down Expand Up @@ -493,6 +509,98 @@ get_negotiated_protocol(SslSocket) ->
_ -> http1
end.

%% @doc Resolve the negotiated protocol after a handshake, carrying the ALPN memo
%% across TLS resumption. On a resumed session ssl:negotiated_protocol/1 reports
%% nothing, so we fall back to `Cached' (snapshotted before the handshake), but
%% only when the session actually resumed - a full handshake that reports no ALPN
%% is a genuine HTTP/1 conn. `Cached' is `recall_alpn(Host, AlpnProtos)' read at
%% the resumption gate. `Resumable' is whether this conn is tied to the resumable
%% ticket source (hackney enabled session_tickets); only then is the memo written,
%% so a custom-ssl_options handshake that does not seed the ticket store cannot
%% poison the shared `{Host, AlpnProtos}' entry a later resumed session reads.
-spec negotiated_protocol(ssl:sslsocket(), string(), list(),
http2 | http1 | none, boolean()) -> http2 | http1.
negotiated_protocol(SslSocket, Host, AlpnProtos, Cached, Resumable) ->
resolve_alpn(ssl:negotiated_protocol(SslSocket), resumed(SslSocket),
Cached, Host, AlpnProtos, Resumable).

%% @doc Pure ALPN decision (exported for tests). See negotiated_protocol/5.
-spec resolve_alpn({ok, binary()} | {error, term()}, boolean(),
http2 | http1 | none, string(), list(), boolean()) ->
http2 | http1.
resolve_alpn({ok, <<"h2">>}, _Resumed, _Cached, Host, AlpnProtos, Resumable) ->
maybe_remember(Resumable, Host, AlpnProtos, http2),
http2;
resolve_alpn({ok, <<"http/1.1">>}, _Resumed, _Cached, Host, AlpnProtos, Resumable) ->
maybe_remember(Resumable, Host, AlpnProtos, http1),
http1;
resolve_alpn({error, protocol_not_negotiated}, true, Cached, _Host, _AlpnProtos, _Resumable)
when Cached =/= none ->
%% Genuinely resumed: ALPN is not re-reported, use the gate-time snapshot.
Cached;
resolve_alpn({error, protocol_not_negotiated}, false, _Cached, Host, AlpnProtos, Resumable) ->
%% Full handshake with no ALPN: a real HTTP/1 conn. Refresh the memo (only for
%% the resumable source) so a stale cached http2 cannot be recalled later.
maybe_remember(Resumable, Host, AlpnProtos, http1),
http1;
resolve_alpn(_Other, _Resumed, _Cached, _Host, _AlpnProtos, _Resumable) ->
%% Defensive: resumed without a snapshot (gate should prevent this) or an
%% unexpected ssl:negotiated_protocol/1 result.
http1.

%% @private Write the ALPN memo only for handshakes tied to the resumable ticket
%% source (hackney's default-config resumption). A non-resumable handshake
%% (custom ssl_options) leaves the shared entry untouched.
maybe_remember(true, Host, AlpnProtos, Proto) -> remember_alpn(Host, AlpnProtos, Proto);
maybe_remember(false, _Host, _AlpnProtos, _Proto) -> ok.

%% @doc Whether the effective opts carry hackney's automatic TLS resumption
%% (`{session_tickets, auto}'), which `effective_opts/3' adds only for the
%% resumable default config. Only such connections are tied to that ticket source
%% and may update the ALPN memo; a caller-supplied `disabled' or `manual' tickets
%% entry is not memo-eligible and must not overwrite the shared entry.
-spec auto_tickets(list()) -> boolean().
auto_tickets(SslOpts) ->
lists:member({session_tickets, auto}, SslOpts).

%% @doc Whether the TLS handshake resumed a session (PSK / abbreviated handshake).
-spec resumed(ssl:sslsocket()) -> boolean().
resumed(SslSocket) ->
case ssl:connection_information(SslSocket, [session_resumption]) of
{ok, [{session_resumption, R}]} -> R =:= true;
_ -> false
end.

%% @doc Recall the protocol learned for `{Host, AlpnProtos}' on a full handshake,
%% or `none' if not cached. `AlpnProtos' is the advertised ALPN list in offered
%% order (order is the client's preference and can change negotiation).
-spec recall_alpn(string(), list()) -> http2 | http1 | none.
recall_alpn(Host, AlpnProtos) ->
try ets:lookup(?ALPN_CACHE, {Host, AlpnProtos}) of
[{_, Proto}] -> Proto;
[] -> none
catch
%% Table absent: hackney used as a library without the app started.
error:badarg -> none
end.

%% @private Cache the protocol for `{Host, AlpnProtos}'. Soft-capped: a generous
%% bound cleared wholesale on overflow. Eviction is not correctness-critical - a
%% cold memo just means the next handshake is full (the gate offers no resumption)
%% and re-learns the protocol.
-spec remember_alpn(string(), list(), http2 | http1) -> ok.
remember_alpn(Host, AlpnProtos, Proto) ->
try
case ets:info(?ALPN_CACHE, size) >= ?ALPN_CACHE_MAX of
true -> _ = ets:delete_all_objects(?ALPN_CACHE);
false -> ok
end,
_ = ets:insert(?ALPN_CACHE, {{Host, AlpnProtos}, Proto}),
ok
catch
error:badarg -> ok
end.

%% @private Convert protocol atom to ALPN protocol identifier
-spec proto_to_alpn(http2 | http1 | http11) -> binary().
proto_to_alpn(http2) -> <<"h2">>;
Expand Down
Loading
Loading