From 0a4ff045f1f86958a536cae9f040972f704455a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Mon, 16 Dec 2024 11:09:19 +0100 Subject: [PATCH] Http2: Ignore RST frames on already-closed streams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some http servers like to send RST frames whenever they send their response. The stream is already closed at that point so it's a little weird, but the RFC doesn't disallow it, so we'll just ignore the frames. Fixes: QTBUG-132124 Change-Id: Ic26e249437b739830935e2f3feec572687579b21 Reviewed-by: Øystein Heskestad Reviewed-by: Timur Pocheptsov (cherry picked from commit 2c71fdf043ca94d1c567f169d51245e2702bec19) Reviewed-by: Qt Cherry-pick Bot --- src/network/access/qhttp2connection.cpp | 3 ++ .../qhttp2connection/tst_qhttp2connection.cpp | 41 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/network/access/qhttp2connection.cpp b/src/network/access/qhttp2connection.cpp index c966d70a350..bd3fd31847f 100644 --- a/src/network/access/qhttp2connection.cpp +++ b/src/network/access/qhttp2connection.cpp @@ -708,6 +708,9 @@ void QHttp2Stream::handleHEADERS(Http2::FrameFlags frameFlags, const HPack::Http void QHttp2Stream::handleRST_STREAM(const Frame &inboundFrame) { + if (m_state == State::Closed) // The stream is already closed, we're not sending anything anyway + return; + transitionState(StateTransition::RST); m_RST_STREAM_received = qFromBigEndian(inboundFrame.dataBegin()); if (isUploadingDATA()) { diff --git a/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp b/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp index a2998ea430e..dfa73a4a457 100644 --- a/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp +++ b/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp @@ -24,6 +24,7 @@ private slots: void testRSTServerSide(); void testRSTClientSide(); void testRSTReplyOnDATAEND(); + void resetAfterClose(); void testBadFrameSize_data(); void testBadFrameSize(); void testDataFrameAfterRSTIncoming(); @@ -420,6 +421,46 @@ void tst_QHttp2Connection::testRSTReplyOnDATAEND() QCOMPARE(errrorServerSpy.count(), 1); } +void tst_QHttp2Connection::resetAfterClose() +{ + 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 }; + + QSignalSpy clientHeaderReceivedSpy{ clientStream, &QHttp2Stream::headersReceived }; + HPack::HttpHeader headers = getRequiredHeaders(); + clientStream->sendHEADERS(headers, true); + + QVERIFY(newIncomingStreamSpy.wait()); + auto *serverStream = newIncomingStreamSpy.front().front().value(); + QCOMPARE(clientStream->streamID(), serverStream->streamID()); + + QSignalSpy errorSpy(clientStream, &QHttp2Stream::errorOccurred); + + const HPack::HttpHeader StatusOKHeaders{ { ":status", "200" } }; + serverStream->sendHEADERS(StatusOKHeaders, true); + + // Write the RST_STREAM frame manually because we guard against sending RST_STREAM on closed + // streams + auto &frameWriter = serverConnection->frameWriter; + frameWriter.start(Http2::FrameType::RST_STREAM, Http2::FrameFlag::EMPTY, + serverStream->streamID()); + frameWriter.append(quint32(Http2::Http2Error::STREAM_CLOSED)); + QVERIFY(frameWriter.write(*serverConnection->getSocket())); + + QVERIFY(clientHeaderReceivedSpy.wait()); + QCOMPARE(clientStream->state(), QHttp2Stream::State::Closed); + + QTest::qWait(10); // Just needs to process events basically + QCOMPARE(errorSpy.count(), 0); +} + void tst_QHttp2Connection::testBadFrameSize_data() { QTest::addColumn("frametype");