Run the HTTP/3 tests against a local server, fix two HTTP/3 bugs

NEWS.md

@@ -17,6 +17,27 @@ unreleased
 - A response that crosses a reset of its stream no longer closes the HTTP/2
   connection. `h2` dropped the header block of that response without decoding
   it, so the next response on the connection failed with COMPRESSION_ERROR.
+- The response to an HTTP/3 streaming upload can be read. After
+  `start_response/1`, `body/1` returned `{error, invalid_state}` and
+  `stream_body/1` returned `{error, no_stream}`: the body went to the
+  `start_response/1` caller as a second reply and was lost. When the response
+  headers arrived before `start_response/1` was called, it could wait forever.
+- HTTP/3 response headers carry one `:status`. quic_h3 passes the status
+  separately and keeps it in the header list, and hackney prepended its own,
+  so every response reached the low-level `{h3, _, {stream_headers, ...}}`
+  consumer with the pseudo-header twice, which RFC 9114 4.3.1 makes a
+  malformed response. Requests through `hackney:request/5` were not affected:
+  pseudo-headers are filtered before the caller sees them.
+- An HTTP/3 connection that goes away reports why. `quic_h3` sends the reason
+  with its close event, and the handler only matched the older shape without
+  one, so the message was dropped: the caller waited out its own timeout and
+  the connection process stayed alive. A handshake that fails on a bad
+  certificate or a TLS alert now comes back as that error instead of
+  `{error, timeout}`.
+- `hackney_h3:connect/3,4` and `hackney_h3:request/5` honor a `cacertfile`
+  option. It was handed to quic, which only takes DER `cacerts`, so it was
+  ignored and verification failed as `{error, timeout}`. `hackney:request/5`
+  was not affected.
 - A request that races a peer-initiated close now returns `{error, closed}`
   instead of `{error, invalid_state}`. A connection that sees the peer close
   stays alive briefly so late calls get an answer, and during that window every

src/hackney_conn.erl

@@ -1297,6 +1297,9 @@ connected({call, From}, stream_body, #conn_data{protocol = http2} = Data) ->
 connected({call, From}, body, #conn_data{protocol = http2} = Data) ->
     handle_h2_read_body(From, Data);
 
+connected({call, From}, body, #conn_data{protocol = http3, h3_streams = Streams} = Data) ->
+    handle_h3_read_body(From, Streams, Data);
+
 connected(EventType, Event, Data) ->
     handle_common(EventType, Event, connected, Data).
 
@@ -1469,7 +1472,7 @@ streaming_body({call, From}, finish_send_body, #conn_data{protocol = http3} = Da
     #conn_data{h3_conn = ConnRef, h3_stream_id = StreamId, h3_streams = Streams} = Data,
     case hackney_h3:finish_send_body(ConnRef, StreamId, Streams) of
         {ok, _} ->
-            {keep_state, Data#conn_data{request_from = From}, [{reply, From, ok}]};
+            {keep_state_and_data, [{reply, From, ok}]};
         {error, Reason} ->
             {next_state, closed, Data, [{reply, From, {error, Reason}}]}
     end;
@@ -1516,11 +1519,21 @@ streaming_body({call, From}, start_response, #conn_data{protocol = http2} = Data
             {next_state, connected, Data, [CancelIdle, {reply, From, {error, no_stream}}]}
     end;
 
-streaming_body({call, From}, start_response, #conn_data{protocol = http3} = Data) ->
-    %% HTTP/3 - wait for response headers via QUIC messages
-    %% Transition to connected and wait for QUIC stream_headers message
-    NewData = Data#conn_data{request_from = From},
-    {next_state, connected, NewData};
+streaming_body({call, From}, start_response,
+               #conn_data{protocol = http3, h3_stream_id = StreamId,
+                          h3_streams = Streams} = Data) ->
+    %% HTTP/3 - the response headers may already be in; otherwise park the
+    %% caller until handle_h3_headers/5 answers it.
+    case maps:get(StreamId, Streams, undefined) of
+        {_, {sending_body, _}} ->
+            {next_state, connected, Data#conn_data{request_from = From}};
+        {_, _Response} ->
+            #conn_data{status = Status, response_headers = Headers} = Data,
+            {next_state, connected, Data,
+             [{reply, From, {ok, Status, Headers, self()}}]};
+        undefined ->
+            {next_state, connected, Data, [{reply, From, {error, no_stream}}]}
+    end;
 
 streaming_body({call, From}, start_response, Data) ->
     %% Transition to receiving state and get response (HTTP/1.1)
@@ -2934,19 +2947,12 @@ h3_ca_opts(SslOpts) ->
         undefined ->
             case proplists:get_value(cacertfile, SslOpts) of
                 undefined -> #{};
-                File -> #{cacerts => cacertfile_ders(File)}
+                File -> #{cacerts => hackney_h3:cacertfile_ders(File)}
             end;
         CACerts ->
             #{cacerts => CACerts}
     end.
 
-%% @private Read a PEM cacertfile into a list of DER certificates.
-cacertfile_ders(File) ->
-    case file:read_file(File) of
-        {ok, Pem} -> [Der || {'Certificate', Der, _} <- public_key:pem_decode(Pem)];
-        {error, _} -> []
-    end.
-
 %% @private Drive QUIC event loop until connected
 wait_h3_connected(ConnRef, Timeout, StartTime) ->
     Elapsed = erlang:monotonic_time(millisecond) - StartTime,
@@ -4038,6 +4044,26 @@ handle_h3_stream_body(From, Streams, Data) ->
             {keep_state_and_data, [{reply, From, {error, no_stream}}]}
     end.
 
+%% @private Handle HTTP/3 body call: the rest of the streaming response in
+%% one binary, returned at once if it already ended or when its FIN arrives.
+handle_h3_read_body(From, Streams, Data) ->
+    case find_streaming_stream(Streams) of
+        {ok, StreamId, {streaming_body, Status, Headers, Buffer, undefined}} ->
+            NewStreamState = {streaming_body_full, Status, Headers, Buffer, From},
+            {keep_state, Data#conn_data{h3_streams = maps:put(StreamId,
+                {undefined, NewStreamState}, Streams)}};
+        {ok, StreamId, {streaming_body_final, _Status, _Headers, Buffer}} ->
+            {keep_state, Data#conn_data{h3_streams = maps:remove(StreamId, Streams)},
+             [{reply, From, {ok, Buffer}}]};
+        {ok, StreamId, {streaming_body_done, _Status, _Headers}} ->
+            {keep_state, Data#conn_data{h3_streams = maps:remove(StreamId, Streams)},
+             [{reply, From, {ok, <<>>}}]};
+        {ok, _StreamId, _OtherState} ->
+            {keep_state_and_data, [{reply, From, {error, not_streaming}}]};
+        none ->
+            {keep_state_and_data, [{reply, From, {error, no_stream}}]}
+    end.
+
 %% @private Find a stream in streaming mode
 find_streaming_stream(Streams) ->
     Result = maps:fold(fun
@@ -4103,29 +4129,24 @@ handle_h3_headers(StreamId, Headers, Fin, Streams, Data) ->
                         status = Status,
                         response_headers = RespHeaders
                     }, [{reply, From, {ok, Status, HeadersList}}]};
-                {From, {sending_body, From}} ->
-                    %% Response arrived while sending body - update state
-                    %% If request_from is set (from start_response), reply to it
+                {_, {sending_body, _}} ->
+                    %% Response to a streamed upload: read it like a
+                    %% request_streaming response, with stream_body/1 or
+                    %% body/1. start_response/1 may already be waiting.
+                    NewStreamState = case Fin of
+                        true -> {streaming_body_done, Status, RespHeaders};
+                        false -> {streaming_body, Status, RespHeaders, <<>>, undefined}
+                    end,
+                    UpdatedStreams = maps:put(StreamId, {undefined, NewStreamState}, Streams),
+                    NewData = Data#conn_data{h3_streams = UpdatedStreams,
+                                             status = Status,
+                                             response_headers = RespHeaders},
                     case Data#conn_data.request_from of
                         undefined ->
-                            %% No one waiting yet - just update state
-                            UpdatedStreams = maps:put(StreamId,
-                                {From, {receiving_body, Status, RespHeaders, <<>>}}, Streams),
-                            {keep_state, Data#conn_data{
-                                h3_streams = UpdatedStreams,
-                                status = Status,
-                                response_headers = RespHeaders
-                            }};
+                            {keep_state, NewData};
                         WaitingFrom ->
-                            %% Someone called start_response - reply with status/headers
-                            UpdatedStreams = maps:put(StreamId,
-                                {WaitingFrom, {receiving_body, Status, RespHeaders, <<>>}}, Streams),
-                            {keep_state, Data#conn_data{
-                                h3_streams = UpdatedStreams,
-                                status = Status,
-                                response_headers = RespHeaders,
-                                request_from = undefined
-                            }, [{reply, WaitingFrom, {ok, Status, HeadersList, self()}}]}
+                            {keep_state, NewData#conn_data{request_from = undefined},
+                             [{reply, WaitingFrom, {ok, Status, HeadersList, self()}}]}
                     end;
                 _ ->
                     %% Unknown stream or wrong state
@@ -4241,6 +4262,18 @@ handle_h3_data(StreamId, RecvData, Fin, Streams, Data) ->
                              [{reply, PendingFrom, {ok, NewBuffer}}]}
                     end
             end;
+        {_, {streaming_body_full, Status, Headers, Acc, From}} ->
+            %% body/1 is waiting for the whole body
+            NewAcc = <<Acc/binary, RecvData/binary>>,
+            case Fin of
+                true ->
+                    {keep_state, Data#conn_data{h3_streams = maps:remove(StreamId, Streams)},
+                     [{reply, From, {ok, NewAcc}}]};
+                false ->
+                    NewStreamState = {streaming_body_full, Status, Headers, NewAcc, From},
+                    {keep_state, Data#conn_data{h3_streams = maps:put(StreamId,
+                        {undefined, NewStreamState}, Streams)}}
+            end;
         {_, {streaming_body_final, Status, Headers, FinalBuffer}} ->
             %% Already got final data, just buffer more (shouldn't happen)
             NewBuffer = <<FinalBuffer/binary, RecvData/binary>>,

src/hackney_h3.erl

@@ -45,6 +45,7 @@
     update_stream_state/3,
     %% Response parsing
     parse_response_headers/1,
+    cacertfile_ders/1,
     %% 0-RTT / session resumption
     early_data_accepted/1,
     get_session_ticket/1,
@@ -849,7 +850,11 @@ handle_info({quic_h3, Conn, connected},
 
 handle_info({quic_h3, Conn, {response, StreamId, Status, Headers}},
             #state{h3_conn = Conn, conn_ref = Ref, owner = Owner} = State) ->
-    Full = [{<<":status">>, integer_to_binary(Status)} | Headers],
+    %% quic_h3 passes the status separately and keeps it in the header list,
+    %% so drop it there before prepending the authoritative one: two
+    %% `:status' make a malformed response (RFC 9114 4.3.1).
+    Rest = [H || {Name, _} = H <- Headers, Name =/= <<":status">>],
+    Full = [{<<":status">>, integer_to_binary(Status)} | Rest],
     Owner ! {h3, Ref, {stream_headers, StreamId, Full, false}},
     {noreply, State};
 
@@ -888,8 +893,20 @@ handle_info({quic_h3, Conn, {early_data_rejected, StreamIds}},
     Owner ! {h3, Ref, {early_data_rejected, StreamIds}},
     {noreply, State};
 
+handle_info({quic_h3, Conn, {closed, Reason}},
+            #state{h3_conn = Conn, conn_ref = Ref, owner = Owner} = State) ->
+    %% quic_h3 reports why the connection went away: `normal' for a local
+    %% close or a drained GOAWAY, `owner_down', an {h3_error, Code, Phrase},
+    %% or whatever QUIC reported. A failure before HTTP/3 comes up (a bad
+    %% certificate, a TLS alert) arrives here too, so pass the reason on
+    %% rather than letting the caller wait out its timeout.
+    Owner ! {h3, Ref, {closed, Reason}},
+    {stop, normal, State};
+
 handle_info({quic_h3, Conn, closed},
             #state{h3_conn = Conn, conn_ref = Ref, owner = Owner} = State) ->
+    %% quic =< 1.10.0 closes without a reason on the local-close and
+    %% QUIC-down paths.
     Owner ! {h3, Ref, {closed, normal}},
     {stop, normal, State};
 
@@ -948,11 +965,13 @@ build_h3_opts(Host, Opts) ->
         disable -> #{};
         Sni -> #{server_name_indication => Sni}
     end,
+    %% quic takes trust anchors as DER `cacerts' only, so a cacertfile is
+    %% decoded here; passed through as is, quic ignored it.
     QuicOpts1 = case maps:get(cacerts, Opts, undefined) of
         undefined ->
             case maps:get(cacertfile, Opts, undefined) of
                 undefined -> QuicOpts0;
-                File -> QuicOpts0#{cacertfile => File}
+                File -> QuicOpts0#{cacerts => cacertfile_ders(File)}
             end;
         CACerts -> QuicOpts0#{cacerts => CACerts}
     end,
@@ -977,6 +996,15 @@ build_h3_opts(Host, Opts) ->
         Settings -> Base#{settings => Settings}
     end.
 
+%% @doc Read a PEM CA file into the DER certificates quic takes as `cacerts'.
+%% An unreadable file gives no anchors, so verification fails closed.
+-spec cacertfile_ders(file:filename_all()) -> [public_key:der_encoded()].
+cacertfile_ders(File) ->
+    case file:read_file(File) of
+        {ok, Pem} -> [Der || {'Certificate', Der, _} <- public_key:pem_decode(Pem)];
+        {error, _} -> []
+    end.
+
 ensure_table() ->
     case ets:whereis(?CONN_TABLE) of
         undefined ->