feat(altsvc): wire parse_and_cache into response path

NEWS.md

@@ -18,6 +18,13 @@ UNRELEASED
 
 ### Bug Fixes
 
+- Wire `hackney_altsvc:parse_and_cache/3` into the response path so
+  server-advertised HTTP/3 endpoints are actually recorded. Previously
+  the cache was only populated by manual `cache/4` calls; the HTTP/3
+  guide claimed automatic discovery but it never fired. Same hook
+  honors RFC 7838 `clear` (invalidates the cached entry) and merges
+  multiple `Alt-Svc` headers per RFC 7230 §3.2.2. Fires on every
+  protocol so the cache TTL stays fresh while h3 is in use.
 - Fix HTTP/2 pooled connections wedging under sustained concurrent load
   (#836). The pool checks out a TCP connection first then upgrades to
   SSL+ALPN; `connected(enter)` armed the 2s pool idle timer while the

src/hackney_altsvc.erl

@@ -80,22 +80,34 @@ parse(Header) when is_binary(Header) ->
     parse_entries(Header, []).
 
 %% @doc Parse Alt-Svc header from response headers and cache if h3 found.
-%% Returns {ok, h3, Port} if HTTP/3 is available, or none.
+%%
+%% Honors RFC 7838: a `clear' value invalidates any cached entry for the
+%% origin. Multiple `Alt-Svc' headers are merged before parsing as if
+%% they had been comma-concatenated (RFC 7230 §3.2.2).
+%%
+%% Returns `{ok, h3, Port}' if HTTP/3 is now cached for the origin,
+%% `cleared' if the cache was invalidated, or `none' otherwise.
 -spec parse_and_cache(Host :: binary() | string(), Port :: inet:port_number(),
                       Headers :: [{binary(), binary()}]) ->
-    {ok, h3, inet:port_number()} | none.
+    {ok, h3, inet:port_number()} | cleared | none.
 parse_and_cache(Host, OrigPort, Headers) ->
-    case find_altsvc_header(Headers) of
-        undefined ->
+    case collect_altsvc_headers(Headers) of
+        <<>> ->
             none;
-        Value ->
-            Entries = parse(Value),
-            case find_h3_entry(Entries) of
-                {ok, H3Port, MaxAge} ->
-                    cache(Host, OrigPort, H3Port, MaxAge),
-                    {ok, h3, H3Port};
-                none ->
-                    none
+        Combined ->
+            case is_clear_directive(Combined) of
+                true ->
+                    clear(Host, OrigPort),
+                    cleared;
+                false ->
+                    Entries = parse(Combined),
+                    case find_h3_entry(Entries) of
+                        {ok, H3Port, MaxAge} ->
+                            cache(Host, OrigPort, H3Port, MaxAge),
+                            {ok, h3, H3Port};
+                        none ->
+                            none
+                    end
             end
     end.
 
@@ -174,12 +186,27 @@ make_key(Host, Port) when is_list(Host) ->
 make_key(Host, Port) when is_binary(Host) ->
     {string:lowercase(Host), Port}.
 
-find_altsvc_header([]) ->
-    undefined;
-find_altsvc_header([{Key, Value} | Rest]) ->
-    case string:lowercase(Key) of
-        <<"alt-svc">> -> Value;
-        _ -> find_altsvc_header(Rest)
+%% Collect every alt-svc header value and join with ", " so multiple
+%% header lines are parsed as a single combined entry list per RFC 7230.
+collect_altsvc_headers(Headers) ->
+    Values = [to_binary(V) || {K, V} <- Headers,
+                              string:lowercase(to_binary(K)) =:= <<"alt-svc">>],
+    join_with_comma(Values).
+
+join_with_comma([]) -> <<>>;
+join_with_comma([V]) -> V;
+join_with_comma([V | Rest]) ->
+    iolist_to_binary([V, ", ", join_with_comma(Rest)]).
+
+to_binary(B) when is_binary(B) -> B;
+to_binary(L) when is_list(L) -> iolist_to_binary(L).
+
+%% RFC 7838 §3: a value of "clear" (case-insensitive) invalidates the
+%% origin's cached alternatives. Surrounding whitespace is allowed.
+is_clear_directive(Value) ->
+    case string:lowercase(string:trim(Value)) of
+        <<"clear">> -> true;
+        _ -> false
     end.
 
 find_h3_entry([]) ->

src/hackney_conn.erl

@@ -1196,6 +1196,7 @@ receiving(internal, do_recv_response_async, Data) ->
     case recv_status_and_headers(DataWithParser) of
         {ok, Status, Headers, NewData} ->
             HeadersList = hackney_headers:to_list(Headers),
+            maybe_record_altsvc(HeadersList, NewData),
             %% Check if this is a redirect and we should handle it
             case maybe_handle_async_redirect(Status, Method, Headers, FollowRedirect) of
                 {redirect, Location} ->
@@ -1656,6 +1657,7 @@ do_recv_response_impl(Data, IncludePid) ->
         {ok, Status, Headers, NewData} ->
             From = NewData#conn_data.request_from,
             HeadersList = hackney_headers:to_list(Headers),
+            maybe_record_altsvc(HeadersList, NewData),
             Reply = case IncludePid of
                 true -> {ok, Status, HeadersList, self()};
                 false -> {ok, Status, HeadersList}
@@ -2485,6 +2487,7 @@ handle_h2_event(_Other, Data) ->
     {keep_state, Data}.
 
 h2_on_response(StreamId, Status, Headers, Data) ->
+    maybe_record_altsvc(Headers, Data),
     #conn_data{h2_streams = Streams} = Data,
     case maps:get(StreamId, Streams, undefined) of
         {From, {sync, waiting_headers}} ->
@@ -2815,6 +2818,7 @@ handle_h3_headers(StreamId, Headers, Fin, Streams, Data) ->
         {ok, Status, RespHeaders} ->
             %% RespHeaders from hackney_h3:parse_response_headers is already a list
             HeadersList = RespHeaders,
+            maybe_record_altsvc(HeadersList, Data),
             case maps:get(StreamId, Streams, undefined) of
                 {From, waiting_headers} ->
                     case Fin of
@@ -3083,3 +3087,14 @@ handle_h3_termination(Error, Data) ->
                     {next_state, closed, NewData, [{reply, From, {error, Error}} | Actions]}
             end
     end.
+
+%% @private Hand response headers to the Alt-Svc cache so server-advertised
+%% HTTP/3 endpoints get recorded for future requests. Fires for every
+%% protocol so the cache TTL stays fresh while h3 is in use and `clear'
+%% directives are honored even on h3 responses.
+maybe_record_altsvc(Headers, #conn_data{host = Host, port = Port})
+  when is_list(Headers) ->
+    _ = catch hackney_altsvc:parse_and_cache(Host, Port, Headers),
+    ok;
+maybe_record_altsvc(_Headers, _Data) ->
+    ok.

test/hackney_altsvc_tests.erl

@@ -79,7 +79,13 @@ cache_test_() ->
                 {"cache and lookup", fun test_cache_lookup/0},
                 {"lookup returns none when not cached", fun test_lookup_none/0},
                 {"clear removes entry", fun test_clear/0},
-                {"parse_and_cache from headers", fun test_parse_and_cache/0}
+                {"parse_and_cache from headers", fun test_parse_and_cache/0},
+                {"clear directive invalidates cached entry",
+                 fun test_parse_and_cache_clear/0},
+                {"multiple Alt-Svc headers are merged",
+                 fun test_parse_and_cache_multi_header/0},
+                {"clear directive case-insensitive with whitespace",
+                 fun test_parse_and_cache_clear_loose/0}
             ]
         }
     }.
@@ -106,6 +112,31 @@ test_parse_and_cache() ->
     %% Verify it's cached
     ?assertEqual({ok, h3, 443}, hackney_altsvc:lookup(<<"cached.example.com">>, 443)).
 
+test_parse_and_cache_clear() ->
+    Host = <<"clear-via-header.example.com">>,
+    hackney_altsvc:cache(Host, 443, 8443, 3600),
+    ?assertEqual({ok, h3, 8443}, hackney_altsvc:lookup(Host, 443)),
+    Headers = [{<<"alt-svc">>, <<"clear">>}],
+    ?assertEqual(cleared,
+                 hackney_altsvc:parse_and_cache(Host, 443, Headers)),
+    ?assertEqual(none, hackney_altsvc:lookup(Host, 443)).
+
+test_parse_and_cache_multi_header() ->
+    %% Two separate Alt-Svc headers must be treated as one combined list.
+    Headers = [{<<"alt-svc">>, <<"h2=\":443\"">>},
+               {<<"alt-svc">>, <<"h3=\":8443\"; ma=600">>}],
+    Result = hackney_altsvc:parse_and_cache(<<"multi.example.com">>, 443, Headers),
+    ?assertEqual({ok, h3, 8443}, Result),
+    ?assertEqual({ok, h3, 8443}, hackney_altsvc:lookup(<<"multi.example.com">>, 443)).
+
+test_parse_and_cache_clear_loose() ->
+    Host = <<"clear-loose.example.com">>,
+    hackney_altsvc:cache(Host, 443, 443, 3600),
+    Headers = [{<<"Alt-Svc">>, <<"  CLEAR  ">>}],
+    ?assertEqual(cleared,
+                 hackney_altsvc:parse_and_cache(Host, 443, Headers)),
+    ?assertEqual(none, hackney_altsvc:lookup(Host, 443)).
+
 %%====================================================================
 %% Blocked Cache Tests
 %%====================================================================

test/hackney_altsvc_wiring_tests.erl

@@ -0,0 +1,87 @@
+%%% -*- erlang -*-
+%%%
+%%% This file is part of hackney released under the Apache 2 license.
+%%% See the NOTICE for more information.
+%%%
+%%% End-to-end test that response-side Alt-Svc parsing actually fires.
+%%% Stands up a tiny cowboy listener that emits a chosen Alt-Svc header
+%%% and asserts the cache reflects what the server said.
+-module(hackney_altsvc_wiring_tests).
+-include_lib("eunit/include/eunit.hrl").
+
+-export([init/2]).  %% cowboy handler callback
+
+-define(PORT, 9982).
+
+setup() ->
+    {ok, _} = application:ensure_all_started(hackney),
+    {ok, _} = application:ensure_all_started(cowboy),
+    Dispatch = cowboy_router:compile([
+        {'_', [{"/[...]", ?MODULE, []}]}
+    ]),
+    {ok, _} = cowboy:start_clear(altsvc_test_http,
+                                 [{port, ?PORT}],
+                                 #{env => #{dispatch => Dispatch}}),
+    hackney_altsvc:init(),
+    hackney_altsvc:clear_all(),
+    ok.
+
+cleanup(_) ->
+    cowboy:stop_listener(altsvc_test_http),
+    hackney_altsvc:clear_all(),
+    ok.
+
+url(Path) ->
+    <<"http://localhost:", (integer_to_binary(?PORT))/binary, Path/binary>>.
+
+altsvc_wiring_test_() ->
+    {setup, fun setup/0, fun cleanup/1,
+     [
+        fun http1_response_populates_cache/0,
+        fun http1_clear_directive_invalidates_cache/0,
+        fun absent_header_leaves_cache_alone/0
+     ]}.
+
+%% ============================================================================
+
+http1_response_populates_cache() ->
+    hackney_altsvc:clear_all(),
+    %% Cowboy handler reads the encoded Alt-Svc value from ?value= and
+    %% emits it on the response.
+    URL = url(<<"/altsvc?value=", (url_encode(<<"h3=\":8443\"; ma=600">>))/binary>>),
+    {ok, 200, _, _} = hackney:request(get, URL, [], <<>>, [{pool, false}]),
+    ?assertEqual({ok, h3, 8443},
+                 hackney_altsvc:lookup(<<"localhost">>, ?PORT)).
+
+http1_clear_directive_invalidates_cache() ->
+    hackney_altsvc:cache(<<"localhost">>, ?PORT, 8443, 3600),
+    ?assertEqual({ok, h3, 8443},
+                 hackney_altsvc:lookup(<<"localhost">>, ?PORT)),
+    URL = url(<<"/altsvc?value=clear">>),
+    {ok, 200, _, _} = hackney:request(get, URL, [], <<>>, [{pool, false}]),
+    ?assertEqual(none, hackney_altsvc:lookup(<<"localhost">>, ?PORT)).
+
+absent_header_leaves_cache_alone() ->
+    hackney_altsvc:cache(<<"localhost">>, ?PORT, 8443, 3600),
+    URL = url(<<"/noaltsvc">>),
+    {ok, 200, _, _} = hackney:request(get, URL, [], <<>>, [{pool, false}]),
+    %% Cache untouched.
+    ?assertEqual({ok, h3, 8443},
+                 hackney_altsvc:lookup(<<"localhost">>, ?PORT)).
+
+%% ============================================================================
+%% cowboy handler — sets Alt-Svc from ?value= query parameter, or none.
+%% ============================================================================
+
+init(Req0, State) ->
+    QS = cowboy_req:parse_qs(Req0),
+    Headers0 = #{<<"content-type">> => <<"text/plain">>},
+    Headers = case proplists:get_value(<<"value">>, QS) of
+        undefined -> Headers0;
+        Value     -> Headers0#{<<"alt-svc">> => Value}
+    end,
+    Req = cowboy_req:reply(200, Headers, <<"ok">>, Req0),
+    {ok, Req, State}.
+
+url_encode(Bin) ->
+    list_to_binary(uri_string:quote(binary_to_list(Bin))).