--- /dev/null
+From 2b1e36e183ce75c224305c7a94457b92f7a5cf58 Mon Sep 17 00:00:00 2001
+From: Mårten Nordheim <marten.nordheim@qt.io>
+Date: Tue, 25 Jun 2024 17:09:35 +0200
+Subject: [PATCH] HTTP2: Delay any communication until encrypted() can be responded to
+
+We have the encrypted() signal that lets users do extra checks on the
+established connection. It is emitted as BlockingQueued, so the HTTP
+thread stalls until it is done emitting. Users can potentially call
+abort() on the QNetworkReply at that point, which is passed as a Queued
+call back to the HTTP thread. That means that any currently queued
+signal emission will be processed before the abort() call is processed.
+
+In the case of HTTP2 it is a little special since it is multiplexed and
+the code is built to start requests as they are available. This means
+that, while the code worked fine for HTTP1, since one connection only
+has one request, it is not working for HTTP2, since we try to send more
+requests in-between the encrypted() signal and the abort() call.
+
+This patch changes the code to delay any communication until the
+encrypted() signal has been emitted and processed, for HTTP2 only.
+It's done by adding a few booleans, both to know that we have to return
+early and so we can keep track of what events arose and what we need to
+resume once enough time has passed that any abort() call must have been
+processed.
+
+Fixes: QTBUG-126610
+Pick-to: 6.5 6.2 5.15 5.12
+Change-Id: Ic25a600c278203256e35f541026f34a8783235ae
+Reviewed-by: Marc Mutz <marc.mutz@qt.io>
+Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
+(cherry picked from commit b1e75376cc3adfc7da5502a277dfe9711f3e0536)
+Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
+(cherry picked from commit 0fb43e4395da34d561814242a0186999e4956e28)
+---
+
+diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp
+index 0abd99b..3631b13 100644
+--- a/src/network/access/qhttp2protocolhandler.cpp
++++ b/src/network/access/qhttp2protocolhandler.cpp
+@@ -303,12 +303,12 @@
+ }
+ }
+
+- if (!prefaceSent && !sendClientPreface())
+- return false;
+-
+ if (!requests.size())
+ return true;
+
++ if (!prefaceSent && !sendClientPreface())
++ return false;
++
+ m_channel->state = QHttpNetworkConnectionChannel::WritingState;
+ // Check what was promised/pushed, maybe we do not have to send a request
+ // and have a response already?
+diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
+index 6766989..1e4161d 100644
+--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
++++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
+@@ -209,6 +209,10 @@
+ bool QHttpNetworkConnectionChannel::sendRequest()
+ {
+ Q_ASSERT(protocolHandler);
++ if (waitingForPotentialAbort) {
++ needInvokeSendRequest = true;
++ return false; // this return value is unused
++ }
+ return protocolHandler->sendRequest();
+ }
+
+@@ -221,21 +225,28 @@
+ void QHttpNetworkConnectionChannel::sendRequestDelayed()
+ {
+ QMetaObject::invokeMethod(this, [this] {
+- Q_ASSERT(protocolHandler);
+ if (reply)
+- protocolHandler->sendRequest();
++ sendRequest();
+ }, Qt::ConnectionType::QueuedConnection);
+ }
+
+ void QHttpNetworkConnectionChannel::_q_receiveReply()
+ {
+ Q_ASSERT(protocolHandler);
++ if (waitingForPotentialAbort) {
++ needInvokeReceiveReply = true;
++ return;
++ }
+ protocolHandler->_q_receiveReply();
+ }
+
+ void QHttpNetworkConnectionChannel::_q_readyRead()
+ {
+ Q_ASSERT(protocolHandler);
++ if (waitingForPotentialAbort) {
++ needInvokeReadyRead = true;
++ return;
++ }
+ protocolHandler->_q_readyRead();
+ }
+
+@@ -1239,7 +1250,18 @@
+ if (!h2RequestsToSend.isEmpty()) {
+ // Similar to HTTP/1.1 counterpart below:
+ const auto &pair = std::as_const(h2RequestsToSend).first();
++ waitingForPotentialAbort = true;
+ emit pair.second->encrypted();
++
++ // We don't send or handle any received data until any effects from
++ // emitting encrypted() have been processed. This is necessary
++ // because the user may have called abort(). We may also abort the
++ // whole connection if the request has been aborted and there is
++ // no more requests to send.
++ QMetaObject::invokeMethod(this,
++ &QHttpNetworkConnectionChannel::checkAndResumeCommunication,
++ Qt::QueuedConnection);
++
+ // In case our peer has sent us its settings (window size, max concurrent streams etc.)
+ // let's give _q_receiveReply a chance to read them first ('invokeMethod', QueuedConnection).
+ }
+@@ -1257,6 +1279,28 @@
+ QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
+ }
+
++
++void QHttpNetworkConnectionChannel::checkAndResumeCommunication()
++{
++ Q_ASSERT(connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2
++ || connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2Direct);
++
++ // Because HTTP/2 requires that we send a SETTINGS frame as the first thing we do, and respond
++ // to a SETTINGS frame with an ACK, we need to delay any handling until we can ensure that any
++ // effects from emitting encrypted() have been processed.
++ // This function is called after encrypted() was emitted, so check for changes.
++
++ if (!reply && h2RequestsToSend.isEmpty())
++ abort();
++ waitingForPotentialAbort = false;
++ if (needInvokeReadyRead)
++ _q_readyRead();
++ if (needInvokeReceiveReply)
++ _q_receiveReply();
++ if (needInvokeSendRequest)
++ sendRequest();
++}
++
+ void QHttpNetworkConnectionChannel::requeueHttp2Requests()
+ {
+ const auto h2RequestsToSendCopy = std::exchange(h2RequestsToSend, {});
+diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h
+index c42290f..061f20f 100644
+--- a/src/network/access/qhttpnetworkconnectionchannel_p.h
++++ b/src/network/access/qhttpnetworkconnectionchannel_p.h
+@@ -74,6 +74,10 @@
+ QAbstractSocket *socket;
+ bool ssl;
+ bool isInitialized;
++ bool waitingForPotentialAbort = false;
++ bool needInvokeReceiveReply = false;
++ bool needInvokeReadyRead = false;
++ bool needInvokeSendRequest = false;
+ ChannelState state;
+ QHttpNetworkRequest request; // current request, only used for HTTP
+ QHttpNetworkReply *reply; // current reply for this request, only used for HTTP
+@@ -146,6 +150,8 @@
+ void closeAndResendCurrentRequest();
+ void resendCurrentRequest();
+
++ void checkAndResumeCommunication();
++
+ bool isSocketBusy() const;
+ bool isSocketWriting() const;
+ bool isSocketWaiting() const;
+diff --git a/tests/auto/network/access/http2/tst_http2.cpp b/tests/auto/network/access/http2/tst_http2.cpp
+index 00efbc9..c02e7b7 100644
+--- a/tests/auto/network/access/http2/tst_http2.cpp
++++ b/tests/auto/network/access/http2/tst_http2.cpp
+@@ -106,6 +106,8 @@
+
+ void duplicateRequestsWithAborts();
+
++ void abortOnEncrypted();
++
+ protected slots:
+ // Slots to listen to our in-process server:
+ void serverStarted(quint16 port);
+@@ -1479,6 +1481,48 @@
+ QCOMPARE(finishedCount, ExpectedSuccessfulRequests);
+ }
+
++void tst_Http2::abortOnEncrypted()
++{
++#if !QT_CONFIG(ssl)
++ QSKIP("TLS support is needed for this test");
++#else
++ clearHTTP2State();
++ serverPort = 0;
++
++ ServerPtr targetServer(newServer(defaultServerSettings, H2Type::h2Direct));
++
++ QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection);
++ runEventLoop();
++
++ nRequests = 1;
++ nSentRequests = 0;
++
++ const auto url = requestUrl(H2Type::h2Direct);
++ QNetworkRequest request(url);
++ request.setAttribute(QNetworkRequest::Http2DirectAttribute, true);
++
++ std::unique_ptr<QNetworkReply> reply{manager->get(request)};
++ reply->ignoreSslErrors();
++ connect(reply.get(), &QNetworkReply::encrypted, reply.get(), [reply = reply.get()](){
++ reply->abort();
++ });
++ connect(reply.get(), &QNetworkReply::errorOccurred, this, &tst_Http2::replyFinishedWithError);
++
++ runEventLoop();
++ STOP_ON_FAILURE
++
++ QCOMPARE(nRequests, 0);
++ QCOMPARE(reply->error(), QNetworkReply::OperationCanceledError);
++
++ const bool res = QTest::qWaitFor(
++ [this, server = targetServer.get()]() {
++ return serverGotSettingsACK || prefaceOK || nSentRequests > 0;
++ },
++ 500);
++ QVERIFY(!res);
++#endif // QT_CONFIG(ssl)
++}
++
+ void tst_Http2::serverStarted(quint16 port)
+ {
+ serverPort = port;
projects / gamesguru / feather.git / commitdiff