HTTP/2: crash after GOAWAY + WINDOW_UPDATE in same TCP segment

[manas-chaudhari]

Environment

• Gun version: master (also affects 2.0.0, 2.1.0, 2.2.0)
• Erlang/OTP: 24+

Description

When gun receives an HTTP/2 GOAWAY frame, it removes streams with ID > LastStreamID from its own streams map. However, it does not remove those streams from the HTTP/2 state machine (cow_http2_machine). The state machine still holds the removed streams along with any buffered send data that could not be sent due to flow control.

If a WINDOW_UPDATE frame arrives in the same TCP segment as the GOAWAY, gun processes both frames in a single parse loop iteration. The GOAWAY handler removes the stream from gun's map. Then the WINDOW_UPDATE handler raises the connection flow control window, causing the state machine to flush buffered send data for the now-removed stream. Gun then tries to look up the stream by ID in its own map and crashes because the stream is no longer there.

Per RFC 7540 Section 6.8, an endpoint that receives a GOAWAY should not send data on streams above LastStreamID, since the server will not process them. The buffered send data for these streams should be discarded when handling the GOAWAY.

Stacktrace

** Reason for termination = error:{badkey,5}
** Stacktrace =
**  [{erlang,map_get,
             [5, #{1 => {stream,...}, 3 => {stream,...}}], ...},
     {gun_http2,get_stream_by_id,2,[{file,"src/gun_http2.erl"},{line,1666}]},
     {gun_http2,send_data,6,[{file,"src/gun_http2.erl"},{line,1292}]},
     {gun_http2,send_data,4,[{file,"src/gun_http2.erl"},{line,1278}]},
     {gun_http2,frame,5,[{file,"src/gun_http2.erl"},{line,359}]},
     {gun_http2,parse,5,[{file,"src/gun_http2.erl"},{line,260}]},
     {gun,handle_common_connected_no_input,4,[{file,"src/gun.erl"},{line,1587}]},
     {gen_statem,loop_state_callback,11,[{file,"gen_statem.erl"},{line,1203}]}]

Reading bottom-up:

• parse/5 (line 260) processes frames one at a time from a TCP data buffer. After each frame, it recurses on the remaining bytes, so GOAWAY and WINDOW_UPDATE from the same segment are handled in sequence within a single handle_common_connected_no_input call.
• frame/5 (line 359) dispatches the WINDOW_UPDATE to cow_http2_machine, which returns {send, SendData} containing the buffered data for stream 5 — a stream that goaway/2 already removed from gun's map.
• send_data/4 (line 1278) iterates over the send list and calls send_data/6 for each entry.
• send_data/6 (line 1292) calls get_stream_by_id(State, 5).
• get_stream_by_id/2 (line 1666) does maps:get(5, Streams) — but stream 5 was removed by goaway_streams/6, so it crashes with {badkey, 5}.

The root cause is in goaway_streams/6: it calls close_stream/3 to notify the caller, but never calls cow_http2_machine:reset_stream/2 to remove the stream from the state machine and discard its buffered data.

Minimal reproduction

Save as repro.escript alongside the cloned gun/ directory:

#!/usr/bin/env escript
%% repro.escript — Standalone reproducer for gun HTTP/2 GOAWAY + WINDOW_UPDATE crash.
%%
%% Run (from directory containing this file and the gun/ repo):
%%   cd gun && make && cd ..
%%   escript repro.escript
%%
%% Expected on UNPATCHED gun:  crash with {badkey, 5}
%% Expected on PATCHED gun:    clean exit, "PASS" printed
%%
%% How it works:
%%
%% We start a minimal HTTP/2 server on a random port and connect gun to it.
%% Gun is configured with initial_connection_window_size=65535 (the HTTP/2
%% default, much smaller than gun's own default of 8,000,000). This means
%% gun can send at most 65,535 bytes of request body before it must wait
%% for a WINDOW_UPDATE from the server.
%%
%% The client sends three POST requests with 30 KB bodies each (streams 1,
%% 3, 5). Total body data = 90 KB, which exceeds the 65,535-byte connection
%% window. Gun sends data for streams 1 and 3, partially sends stream 5,
%% and buffers the remainder (~26 KB) inside cow_http2_machine.
%%
%% The server then sends 200 responses for streams 1 and 3, followed by
%% GOAWAY(LastStreamID=3) and a connection-level WINDOW_UPDATE(65535) in a
%% single TCP write. Because both frames arrive in the same TCP segment,
%% gun processes them in one parse loop iteration:
%%
%%   1. GOAWAY: gun removes stream 5 from its streams map.
%%   2. WINDOW_UPDATE: cow_http2_machine raises the connection window and
%%      tries to flush the buffered data for stream 5 — but gun already
%%      removed it, causing a {badkey, 5} crash in get_stream_by_id/2.

main(_) ->
    code:add_pathsz(["gun/ebin" | filelib:wildcard("gun/deps/*/ebin")]),
    application:ensure_all_started(gun),
    Parent = self(),
    ServerPid = spawn_link(fun() -> server(Parent) end),
    Port = receive {ServerPid, P} -> P end,
    %% Connect with small connection window so gun exhausts it quickly.
    %% 65535 is the HTTP/2 default; gun normally overrides this to 8,000,000.
    %% Using 65535 also suppresses the connection-level WINDOW_UPDATE that
    %% gun would otherwise send in its preface, making the handshake
    %% frame sequence deterministic.
    {ok, ConnPid} = gun:open("localhost", Port, #{
        protocols => [http2],
        retry => 0,
        http2_opts => #{
            initial_connection_window_size => 65535,
            initial_stream_window_size => 65535
        }
    }),
    ConnRef = monitor(process, ConnPid),
    {ok, http2} = gun:await_up(ConnPid),
    receive {ServerPid, handshake_completed} -> ok end,
    %% Send three 30 KB POST requests. Total = 90 KB > 65535-byte window.
    %% Stream 5 will have ~26 KB buffered inside cow_http2_machine.
    Body = binary:copy(<<$x>>, 30720),
    Hdrs = [{<<"content-type">>, <<"application/octet-stream">>}],
    _ = gun:post(ConnPid, "/req1", Hdrs, Body),
    _ = gun:post(ConnPid, "/req2", Hdrs, Body),
    _ = gun:post(ConnPid, "/req3", Hdrs, Body),
    %% Wait for gun to process GOAWAY + WINDOW_UPDATE: crash or clean exit.
    receive
        {'DOWN', ConnRef, process, ConnPid, normal} ->
            io:format("PASS: gun exited normally (no crash).~n"),
            halt(0);
        {'DOWN', ConnRef, process, ConnPid, Reason} ->
            io:format("FAIL: gun crashed: ~p~n", [Reason]),
            halt(1)
    after 15000 ->
        io:format("FAIL: timeout waiting for gun to exit.~n"),
        halt(2)
    end.

server(Parent) ->
    {ok, LSock} = gen_tcp:listen(0, [binary, {packet, raw},
        {active, false}, {reuseaddr, true}]),
    {ok, Port} = inet:port(LSock),
    Parent ! {self(), Port},
    {ok, Sock} = gen_tcp:accept(LSock, 5000),
    %% --- HTTP/2 handshake ---
    %% Send empty SETTINGS (all defaults).
    ok = gen_tcp:send(Sock, iolist_to_binary(cow_http2:settings(#{}))),
    %% Read client connection preface (24-byte magic).
    {ok, <<"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n">>} = gen_tcp:recv(Sock, 24, 5000),
    %% Read client SETTINGS (9-byte header, 0-byte payload — gun sends
    %% no custom settings when windows are at HTTP/2 defaults).
    {ok, <<0:24, 4:8, 0:8, 0:32>>} = gen_tcp:recv(Sock, 9, 5000),
    %% ACK client's SETTINGS.
    ok = gen_tcp:send(Sock, iolist_to_binary(cow_http2:settings_ack())),
    %% Read client's SETTINGS_ACK.
    {ok, <<0:24, 4:8, 1:8, 0:32>>} = gen_tcp:recv(Sock, 9, 5000),
    Parent ! {self(), handshake_completed},
    %% --- Trigger the crash ---
    %% Let gun exhaust the connection window with POST body data.
    timer:sleep(500),
    %% Send 200 responses for streams 1 and 3.
    {HB1, HS0} = cow_hpack:encode([{<<":status">>, <<"200">>}]),
    {HB2, _} = cow_hpack:encode([{<<":status">>, <<"200">>}], HS0),
    ok = gen_tcp:send(Sock, iolist_to_binary([
        cow_http2:headers(1, fin, HB1),
        cow_http2:headers(3, fin, HB2)
    ])),
    timer:sleep(200),
    %% GOAWAY(3) + WINDOW_UPDATE(65535) in one TCP write — crash trigger.
    GoAway = iolist_to_binary(cow_http2:goaway(3, no_error, <<>>)),
    WU = iolist_to_binary(cow_http2:window_update(65535)),
    ok = gen_tcp:send(Sock, <<GoAway/binary, WU/binary>>),
    %% Keep socket open for gun to process both frames.
    timer:sleep(3000),
    gen_tcp:close(Sock),
    gen_tcp:close(LSock).

Steps:

1. Clone gun at master: git clone https://github.com/ninenines/gun
2. Build: cd gun && make && cd ..
3. Save repro.escript alongside the gun/ directory
4. Run: escript repro.escript

Expected output on unpatched gun: FAIL: gun crashed: {badkey, 5}

Proposed fix

goaway_streams should call cow_http2_machine:reset_stream/2 for each stream it removes, so the state machine discards their buffered send data before any subsequent WINDOW_UPDATE can trigger a flush. This requires threading the HTTP2Machine state through goaway_streams and updating it in the goaway/2 caller.

[essen]

Thanks. Perhaps there should be a function to tell the machine about the goaway and let it clean up fully at once instead of calling reset_stream for each stream one by one.

[manas-chaudhari]

Created #365

Thanks for the suggestion. I explored adding a batch function to cow_http2_machine but decided to keep the per-stream reset_stream approach for now.

A goaway/2 function in the machine would need to embed the filtering logic (which streams to cancel), but that logic differs between gun and cowboy — gun cancels odd (client-initiated) streams, cowboy cancels even (server-initiated) streams. The decision of which streams to remove is a caller concern.

A simpler alternative would be a batch reset_streams/2 that takes a list of stream IDs, but that's just a convenience wrapper around the existing reset_stream/2 loop. It doesn't fix a bug or remove duplication — the filtering still lives in the caller.

Note: cowboy's goaway_streams has the same issue — it doesn't call reset_stream either. It may be worth applying the same fix there.

Also noticed both comments are wrong: gun says "Cancel server-initiated streams" but cancels client-initiated (odd), cowboy says "Cancel client-initiated streams" but cancels server-initiated (even). The code is correct in both cases.

[manas-chaudhari]

@essen can you help review the PR? FYI we are seeing crashes due to this bug periodically. Hence, I want to prioritize it. Happy to address any feedback.

[essen]

I'm on vacation. Outside of that my priority in the foreseeable future is HTTP/3+WT. Your PR should work fine in the meantime.