Http2: Explicitly send RST_STREAM on cancelled request

It will do this when it gets deleted, but due to deleteLater just adding
an event to the event queue the events that are ahead in the queue may
use the stream in question. This would lead to a variant of
'stream not found', or specifically in the case of the bugreport, a
'HEADERS on non-existent stream' stream error.

Amends 6b4e11e63ead46dde5c1002c123ca964bb6aa342

Fixes: QTBUG-137427
Pick-to: 6.10 6.9
Change-Id: I5f2b2d5660866f1ad12aaafbb4e572b08ed5a6e4
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
This commit is contained in:
Mårten Nordheim 2025-06-05 12:12:56 +02:00
parent 97fe141f25
commit 904aec2f37
2 changed files with 65 additions and 0 deletions

View File

@ -265,6 +265,7 @@ bool QHttp2ProtocolHandler::tryRemoveReply(QHttpNetworkReply *reply)
{ {
QHttp2Stream *stream = streamIDs.take(reply); QHttp2Stream *stream = streamIDs.take(reply);
if (stream) { if (stream) {
stream->sendRST_STREAM(stream->isUploadingDATA() ? Http2::CANCEL : Http2::HTTP2_NO_ERROR);
requestReplyPairs.remove(stream); requestReplyPairs.remove(stream);
stream->deleteLater(); stream->deleteLater();
return true; return true;

View File

@ -88,6 +88,7 @@ private slots:
void goaway(); void goaway();
void earlyResponse(); void earlyResponse();
void earlyError(); void earlyError();
void abortReply();
void connectToHost_data(); void connectToHost_data();
void connectToHost(); void connectToHost();
void maxFrameSize(); void maxFrameSize();
@ -774,6 +775,69 @@ void tst_Http2::earlyError()
QTRY_VERIFY(serverGotSettingsACK); QTRY_VERIFY(serverGotSettingsACK);
} }
/*
As above this test relies a bit on timing so we are
using QHttpNetworkRequest directly.
*/
void tst_Http2::abortReply()
{
clearHTTP2State();
serverPort = 0;
const auto serverConnectionType = defaultConnectionType() == H2Type::h2c ? H2Type::h2Direct
: H2Type::h2Alpn;
ServerPtr targetServer(newServer(defaultServerSettings, serverConnectionType));
QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection);
runEventLoop();
QVERIFY(serverPort != 0);
nRequests = 1;
// SETUP create QHttpNetworkConnection primed for http2 usage
const auto connectionType = serverConnectionType == H2Type::h2Direct
? QHttpNetworkConnection::ConnectionTypeHTTP2Direct
: QHttpNetworkConnection::ConnectionTypeHTTP2;
QHttpNetworkConnection connection(1, "127.0.0.1", serverPort, true, false, nullptr,
connectionType);
QSslConfiguration config = QSslConfiguration::defaultConfiguration();
config.setAllowedNextProtocols({"h2"});
connection.setSslConfiguration(config);
connection.ignoreSslErrors();
// SETUP manually setup the QHttpNetworkRequest
QHttpNetworkRequest req;
req.setSsl(true);
req.setHTTP2Allowed(true);
if (defaultConnectionType() == H2Type::h2c)
req.setH2cAllowed(true);
req.setOperation(QHttpNetworkRequest::Post);
req.setUrl(requestUrl(defaultConnectionType()));
// ^ All the above is set-up, the real code starts below v
std::unique_ptr<QHttpNetworkReply> reply{connection.sendRequest(req)};
QVERIFY(reply);
QSemaphore sem;
QObject::connect(reply.get(), &QHttpNetworkReply::requestSent, reply.get(), [&](){
reply.reset();
sem.release();
});
// failOnWarning doesn't work for qCritical, so we set this env-var:
const char envvar[] = "QT_FATAL_CRITICALS";
auto restore = qScopeGuard([envvar, prev = qgetenv(envvar)]() {
qputenv(envvar, prev);
});
qputenv(envvar, "1");
QTest::failOnWarning(QRegularExpression("HEADERS on invalid stream"));
QVERIFY(QTest::qWaitFor([&sem]() { return sem.tryAcquire(); }));
using namespace std::chrono_literals;
// Process some extra events in case they trigger an error:
QTest::qWait(100ms);
}
void tst_Http2::connectToHost_data() void tst_Http2::connectToHost_data()
{ {
// The attribute to set on a new request: // The attribute to set on a new request: