From 4a95b7fc6022d9d13098d20df8b5fc32b82404c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Mon, 28 Apr 2025 20:38:31 +0200 Subject: [PATCH] Http2: process HEADERS on RSTed streams If we RST a stream then we may still have to process HEADER frames for those streams (at least for a while) that was likely already in-transit. This is due to HPACK, which manages header compression state. Missing a frame desynchronizes the server and client, eventually leading to an error where one party is referring to a header the other party doesn't have. Fixes: QTBUG-135800 Pick-to: 6.9 6.8 Change-Id: I5b4b510225ce6737fec7128c4bad38219404b196 Reviewed-by: Timur Pocheptsov (cherry picked from commit 9224fdd3f4cccce7a8123ab398f6ff8745d759ce) Reviewed-by: Matthias Rauter Reviewed-by: Edward Welbourne --- src/network/access/qhttp2connection.cpp | 2 +- .../qhttp2connection/tst_qhttp2connection.cpp | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/network/access/qhttp2connection.cpp b/src/network/access/qhttp2connection.cpp index cac6be7faa0..090468c39a9 100644 --- a/src/network/access/qhttp2connection.cpp +++ b/src/network/access/qhttp2connection.cpp @@ -1875,7 +1875,7 @@ void QHttp2Connection::handleContinuedHEADERS() const auto streamIt = m_streams.constFind(streamID); if (firstFrameType == FrameType::HEADERS) { - if (streamIt != m_streams.cend()) { + if (streamIt != m_streams.cend() && !streamIt.value()->wasReset()) { QHttp2Stream *stream = streamIt.value(); if (stream->state() != QHttp2Stream::State::HalfClosedLocal && stream->state() != QHttp2Stream::State::ReservedRemote diff --git a/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp b/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp index 440c34b6b9f..aca66f2b8f7 100644 --- a/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp +++ b/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp @@ -30,6 +30,7 @@ private slots: void testBadFrameSize(); void testDataFrameAfterRSTIncoming(); void testDataFrameAfterRSTOutgoing(); + void headerFrameAfterRSTOutgoing(); void connectToServer(); void WINDOW_UPDATE(); void testCONTINUATIONFrame(); @@ -727,6 +728,53 @@ void tst_QHttp2Connection::testDataFrameAfterRSTOutgoing() QVERIFY(closedServerSpy.wait()); } +void tst_QHttp2Connection::headerFrameAfterRSTOutgoing() +{ + auto [client, server] = makeFakeConnectedSockets(); + auto *connection = makeHttp2Connection(client.get(), {}, Client); + auto *serverConnection = makeHttp2Connection(server.get(), {}, Server); + + QHttp2Stream *clientStream = connection->createStream().unwrap(); + QVERIFY(clientStream); + QVERIFY(waitForSettingsExchange(connection, serverConnection)); + + QSignalSpy newIncomingStreamSpy{ serverConnection, &QHttp2Connection::newIncomingStream }; + HPack::HttpHeader headers = getRequiredHeaders(); + + // Send some headers to let the server know about the stream + clientStream->sendHEADERS(headers, false); + + // Wait for the stream on the server side + QVERIFY(newIncomingStreamSpy.wait()); + auto *serverStream = newIncomingStreamSpy.front().front().value(); + newIncomingStreamSpy.clear(); + + QSignalSpy serverRSTReceivedSpy{ serverStream, &QHttp2Stream::rstFrameReceived }; + + // Send an RST frame from the client, but we don't process it yet + clientStream->sendRST_STREAM(Http2::CANCEL); + + // The server sends a reply, not knowing about the inbound RST frame + const HPack::HttpHeader StandardReply{ { ":status", "200" }, { "x-whatever", "some info" } }; + serverStream->sendHEADERS(StandardReply, true); + + // With the bug in QTBUG-135800 we would ignore the RST frame, not processing it at all. + // This caused the HPACK lookup tables to be out of sync between server and client, eventually + // causing an error on Qt's side. + QVERIFY(serverRSTReceivedSpy.wait()); + + // Create a new stream then send and handle a new request! + QHttp2Stream *clientStream2 = connection->createStream().unwrap(); + QSignalSpy client2HeaderReceivedSpy{ clientStream2, &QHttp2Stream::headersReceived }; + QSignalSpy client2ErrorOccurredSpy{ clientStream, &QHttp2Stream::errorOccurred }; + clientStream2->sendHEADERS(headers, true); + QVERIFY(newIncomingStreamSpy.wait()); + QHttp2Stream *serverStream2 = newIncomingStreamSpy.front().front().value(); + serverStream2->sendHEADERS(StandardReply, true); + QVERIFY(client2HeaderReceivedSpy.wait()); + QCOMPARE(client2ErrorOccurredSpy.count(), 0); +} + void tst_QHttp2Connection::connectToServer() { auto [client, server] = makeFakeConnectedSockets();