Handle closed QUIC handshakes gracefully

src/cowboy.erl

@@ -120,12 +120,18 @@ start_quic(Ref, TransOpts, ProtoOpts) ->
 				%% the connection otherwise streams may come in before
 				%% the controlling process is changed and messages will
 				%% not be sent to the correct process.
-				{ok, Conn} = quicer:handshake(Conn),
-				process_flag(trap_exit, true), %% @todo Only if supervisor though.
-				try cowboy_http3:init(Parent, Ref, Conn, ProtoOpts)
-				catch
-					exit:{shutdown,_} -> ok;
-					C:E:S -> log(error, "CRASH ~p:~p:~p", [C,E,S], ProtoOpts)
+				case quicer:handshake(Conn) of
+					{ok, Conn1} ->
+						process_flag(trap_exit, true), %% @todo Only if supervisor though.
+						try cowboy_http3:init(Parent, Ref, Conn1, ProtoOpts)
+						catch
+							exit:{shutdown,_} -> ok;
+							C:E:S -> log(error, "CRASH ~p:~p:~p", [C,E,S], ProtoOpts)
+						end;
+					{error, closed} ->
+						ok;
+					{error, Reason} ->
+						log(warning, "QUIC handshake failed: ~p", [Reason], ProtoOpts)
 				end
 			end),
 			ok = quicer:controlling_process(Conn, Pid),

test/handlers/error_forward_h.erl

@@ -0,0 +1,42 @@
+%% Copyright (c) Loïc Hoguin <essen@ninenines.eu>
+%%
+%% Permission to use, copy, modify, and/or distribute this software for any
+%% purpose with or without fee is hereby granted, provided that the above
+%% copyright notice and this permission notice appear in all copies.
+%%
+%% THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+%% WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+%% MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+%% ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+%% WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+%% ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+%% OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+-module(error_forward_h).
+-behaviour(gen_event).
+
+-export([init/1]).
+-export([handle_event/2]).
+-export([handle_call/2]).
+-export([handle_info/2]).
+-export([terminate/2]).
+-export([code_change/3]).
+
+init(Owner) ->
+	{ok, Owner}.
+
+handle_event(Event, Owner) ->
+	Owner ! {cowboy_error_event, Event},
+	{ok, Owner}.
+
+handle_call(_, State) ->
+	{ok, ok, State}.
+
+handle_info(_, State) ->
+	{ok, State}.
+
+terminate(_, _) ->
+	ok.
+
+code_change(_, State, _) ->
+	{ok, State}.

test/rfc9114_SUITE.erl

@@ -66,6 +66,26 @@ alpn_error(Config) ->
 			#{alpn => ["h2"], verify => none}, 5000),
 	ok.
 
+handshake_close_does_not_crash_acceptor(Config) ->
+	doc("A client that disconnects during handshake must not crash the QUIC acceptor. "
+		"(minimal repro for start_quic/3 handshake-close handling)"),
+	ok = gen_event:add_sup_handler(error_logger, error_forward_h, self()),
+	try
+		do_flush_error_events(),
+		do_trigger_handshake_closes(Config, 200),
+		Deadline = erlang:monotonic_time(millisecond) + 2000,
+		false = do_wait_for_start_quic_badmatch(Deadline),
+		%% Repro assertion: listener must still accept a fresh connection.
+		{ok, Conn1} = quicer:connect("localhost", config(port, Config),
+			#{alpn => ["h3"], verify => none}, 5000),
+		{ok, _ControlRef, _Settings} = do_wait_settings(Conn1),
+		ok = quicer:close_connection(Conn1),
+		ok
+	after
+		_ = catch gen_event:delete_handler(error_logger, error_forward_h, ok),
+		ok
+	end.
+
 %% @todo 3.2. Connection Establishment
 %% After the QUIC connection is established, a SETTINGS frame MUST be sent by each endpoint as the initial frame of their respective HTTP control stream.
 
@@ -2315,6 +2335,57 @@ do_connect(Config, Opts) ->
 		settings => Settings
 	}.
 
+do_flush_conn_messages(Conn) ->
+	receive
+		{quic, _, Conn, _} ->
+			do_flush_conn_messages(Conn)
+	after 0 ->
+		ok
+	end.
+
+do_trigger_handshake_closes(_Config, 0) ->
+	ok;
+do_trigger_handshake_closes(Config, N) ->
+	{ok, Conn} = quicer:async_connect("localhost", config(port, Config),
+		#{alpn => ["h3"], verify => none}),
+	ok = quicer:async_close_connection(Conn),
+	do_flush_conn_messages(Conn),
+	do_trigger_handshake_closes(Config, N - 1).
+
+do_flush_error_events() ->
+	receive
+		{cowboy_error_event, _} ->
+			do_flush_error_events()
+	after 0 ->
+		ok
+	end.
+
+do_wait_for_start_quic_badmatch(Deadline) ->
+	Timeout = Deadline - erlang:monotonic_time(millisecond),
+	if
+		Timeout =< 0 ->
+			false;
+		true ->
+			receive
+				{cowboy_error_event, Event} ->
+					case do_is_start_quic_badmatch(Event) of
+						true -> true;
+						false -> do_wait_for_start_quic_badmatch(Deadline)
+					end
+			after Timeout ->
+				false
+			end
+	end.
+
+do_is_start_quic_badmatch(Event) ->
+	EventBin = iolist_to_binary(io_lib:format("~tp", [Event])),
+	binary:match(EventBin, <<"start_quic">>) =/= nomatch
+		andalso binary:match(EventBin, <<"badmatch">>) =/= nomatch
+		andalso (
+			binary:match(EventBin, <<"error,closed">>) =/= nomatch
+			orelse binary:match(EventBin, <<"error, closed">>) =/= nomatch
+		).
+
 do_wait_settings(Conn) ->
 	receive
 		{quic, new_stream, StreamRef, #{flags := Flags}} ->