diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp index d9341dc6435..419c1e2180d 100644 --- a/src/network/access/qhttp2protocolhandler.cpp +++ b/src/network/access/qhttp2protocolhandler.cpp @@ -304,12 +304,12 @@ bool QHttp2ProtocolHandler::sendRequest() } } - 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 8688e4b8d71..98961fc2fe0 100644 --- a/src/network/access/qhttpnetworkconnectionchannel.cpp +++ b/src/network/access/qhttpnetworkconnectionchannel.cpp @@ -234,6 +234,10 @@ void QHttpNetworkConnectionChannel::abort() bool QHttpNetworkConnectionChannel::sendRequest() { Q_ASSERT(protocolHandler); + if (waitingForPotentialAbort) { + needInvokeSendRequest = true; + return false; // this return value is unused + } return protocolHandler->sendRequest(); } @@ -246,21 +250,28 @@ bool QHttpNetworkConnectionChannel::sendRequest() 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(); } @@ -1290,7 +1301,18 @@ void QHttpNetworkConnectionChannel::_q_encrypted() 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). } @@ -1308,6 +1330,28 @@ void QHttpNetworkConnectionChannel::_q_encrypted() 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 853b647ecc5..9c79566d5b7 100644 --- a/src/network/access/qhttpnetworkconnectionchannel_p.h +++ b/src/network/access/qhttpnetworkconnectionchannel_p.h @@ -75,6 +75,10 @@ public: QIODevice *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 @@ -147,6 +151,8 @@ public: 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 396a6f2fda7..17cb62ac246 100644 --- a/tests/auto/network/access/http2/tst_http2.cpp +++ b/tests/auto/network/access/http2/tst_http2.cpp @@ -108,6 +108,8 @@ private slots: void duplicateRequestsWithAborts(); + void abortOnEncrypted(); + protected slots: // Slots to listen to our in-process server: void serverStarted(quint16 port); @@ -1545,6 +1547,48 @@ void tst_Http2::duplicateRequestsWithAborts() 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 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;