1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
|
From 02cb8d764673b9c4aa268aafd8370041b5afe231 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= <marten.nordheim@qt.io>
Date: Thu, 5 Jun 2025 10:57:07 +0200
Subject: [PATCH 24/31] Http2: fix handling incoming frames on locally reset
stream
After some of the RST stream handling was updated to more closely
follow the RFC it was accidentally not updating the handleHEADERS
function, and the handleDATA function was handled incorrectly leading
to a potential nullptr dereference.
Amends d17d260948e16549d82f1fdd4dec98d246b0622e.
Change-Id: I345448efd7da92f4f74033b03a5c040b5db9d271
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
(cherry picked from commit 8d5db24c14d5c7ea8a2abf55e4fd88472dd9ca8b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 4235f0587b144ef90e4736861b4b25371efc49b9)
---
src/network/access/qhttp2connection.cpp | 27 ++++++++++++-------
src/network/access/qhttp2protocolhandler.cpp | 5 +++-
.../qhttp2connection/tst_qhttp2connection.cpp | 11 ++++++++
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/src/network/access/qhttp2connection.cpp b/src/network/access/qhttp2connection.cpp
index 1e864041f62..5abec0c0273 100644
--- a/src/network/access/qhttp2connection.cpp
+++ b/src/network/access/qhttp2connection.cpp
@@ -1384,13 +1384,16 @@ void QHttp2Connection::handleDATA()
if (isInvalidStream(streamID))
return connectionError(ENHANCE_YOUR_CALM, "DATA on invalid stream");
- // RFC9113, 6.1: If a DATA frame is received whose stream is not in the "open" or
- // "half-closed (local)" state, the recipient MUST respond with a stream error.
- auto stream = getStream(streamID);
- if (stream->state() == QHttp2Stream::State::HalfClosedRemote
- || stream->state() == QHttp2Stream::State::Closed) {
- return stream->streamError(Http2Error::STREAM_CLOSED,
- QLatin1String("Data on closed stream"));
+ QHttp2Stream *stream = nullptr;
+ if (!streamWasResetLocally(streamID)) {
+ stream = getStream(streamID);
+ // RFC9113, 6.1: If a DATA frame is received whose stream is not in the "open" or
+ // "half-closed (local)" state, the recipient MUST respond with a stream error.
+ if (stream->state() == QHttp2Stream::State::HalfClosedRemote
+ || stream->state() == QHttp2Stream::State::Closed) {
+ return stream->streamError(Http2Error::STREAM_CLOSED,
+ QLatin1String("Data on closed stream"));
+ }
}
if (qint32(inboundFrame.payloadSize()) > sessionReceiveWindowSize) {
@@ -1403,9 +1406,8 @@ void QHttp2Connection::handleDATA()
sessionReceiveWindowSize -= inboundFrame.payloadSize();
- auto it = m_streams.constFind(streamID);
- if (it != m_streams.cend() && it.value())
- it.value()->handleDATA(inboundFrame);
+ if (stream)
+ stream->handleDATA(inboundFrame);
if (inboundFrame.flags().testFlag(FrameFlag::END_STREAM))
emit receivedEND_STREAM(streamID);
@@ -1451,6 +1453,11 @@ void QHttp2Connection::handleHEADERS()
qCDebug(qHttp2ConnectionLog, "[%p] Created new incoming stream %d", this, streamID);
emit newIncomingStream(newStream);
+ } else if (streamWasResetLocally(streamID)) {
+ qCDebug(qHttp2ConnectionLog,
+ "[%p] Received HEADERS on previously locally reset stream %d (must process but ignore)",
+ this, streamID);
+ // nop
} else if (auto it = m_streams.constFind(streamID); it == m_streams.cend()) {
// RFC 9113, 6.2: HEADERS frames MUST be associated with a stream.
// A connection error is not required but it seems to be the right thing to do.
diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp
index a99921f5288..84bf5b1a78d 100644
--- a/src/network/access/qhttp2protocolhandler.cpp
+++ b/src/network/access/qhttp2protocolhandler.cpp
@@ -310,7 +310,8 @@ void QHttp2ProtocolHandler::handleHeadersReceived(const HPack::HttpHeader &heade
auto &requestPair = requestReplyPairs[stream];
auto *httpReply = requestPair.second;
auto &httpRequest = requestPair.first;
- Q_ASSERT(httpReply || stream->state() == QHttp2Stream::State::ReservedRemote);
+ if (!httpReply && (!stream || stream->state() != QHttp2Stream::State::ReservedRemote))
+ return;
auto *httpReplyPrivate = httpReply->d_func();
@@ -393,6 +394,8 @@ void QHttp2ProtocolHandler::handleDataReceived(const QByteArray &data, bool endS
QHttp2Stream *stream = qobject_cast<QHttp2Stream *>(sender());
auto &httpPair = requestReplyPairs[stream];
auto *httpReply = httpPair.second;
+ if (!httpReply)
+ return;
Q_ASSERT(!stream->isPromisedStream());
if (!data.isEmpty() && !httpPair.first.d->needResendWithCredentials) {
diff --git a/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp b/tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp
index aca66f2b8f7..90ac4b6fefa 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_data();
void headerFrameAfterRSTOutgoing();
void connectToServer();
void WINDOW_UPDATE();
@@ -728,8 +729,16 @@ void tst_QHttp2Connection::testDataFrameAfterRSTOutgoing()
QVERIFY(closedServerSpy.wait());
}
+void tst_QHttp2Connection::headerFrameAfterRSTOutgoing_data()
+{
+ QTest::addColumn<bool>("deleteStream");
+ QTest::addRow("retain-stream") << false;
+ QTest::addRow("delete-stream") << true;
+}
+
void tst_QHttp2Connection::headerFrameAfterRSTOutgoing()
{
+ QFETCH(const bool, deleteStream);
auto [client, server] = makeFakeConnectedSockets();
auto *connection = makeHttp2Connection(client.get(), {}, Client);
auto *serverConnection = makeHttp2Connection(server.get(), {}, Server);
@@ -753,6 +762,8 @@ void tst_QHttp2Connection::headerFrameAfterRSTOutgoing()
// Send an RST frame from the client, but we don't process it yet
clientStream->sendRST_STREAM(Http2::CANCEL);
+ if (deleteStream)
+ delete std::exchange(clientStream, nullptr);
// The server sends a reply, not knowing about the inbound RST frame
const HPack::HttpHeader StandardReply{ { ":status", "200" }, { "x-whatever", "some info" } };
--
2.50.1
|