From 9078b41dde7eddd449d77399dbd0102f656cafab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Thu, 3 Feb 2022 15:08:08 +0100 Subject: [PATCH] QHttp: Move the emission of requestSent Moved to after the header is actually written, not just generated. For requests with data (put/post) we may have to wait for a callback before the data to write is available. Since we then delay writing the header as well it would be disingenuous to emit requestSent(). Pick-to: 6.3 Change-Id: I76c2d40ca48faaa1f6730ce8b3d5a8a4c3156f8f Reviewed-by: Timur Pocheptsov --- src/network/access/qhttpprotocolhandler.cpp | 3 +- .../qnetworkreply/tst_qnetworkreply.cpp | 29 ++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/network/access/qhttpprotocolhandler.cpp b/src/network/access/qhttpprotocolhandler.cpp index 488c226459..973c200736 100644 --- a/src/network/access/qhttpprotocolhandler.cpp +++ b/src/network/access/qhttpprotocolhandler.cpp @@ -319,7 +319,6 @@ bool QHttpProtocolHandler::sendRequest() #else m_header = QHttpNetworkRequestPrivate::header(m_channel->request, false); #endif - QMetaObject::invokeMethod(m_reply, "requestSent", Qt::QueuedConnection); // flushing is dangerous (QSslSocket calls transmit which might read or error) // m_socket->flush(); @@ -335,6 +334,7 @@ bool QHttpProtocolHandler::sendRequest() } else { // no data to send: just send the HTTP headers m_socket->write(qExchange(m_header, {})); + QMetaObject::invokeMethod(m_reply, "requestSent", Qt::QueuedConnection); m_channel->state = QHttpNetworkConnectionChannel::WaitingState; // now wait for response sendRequest(); //recurse } @@ -409,6 +409,7 @@ bool QHttpProtocolHandler::sendRequest() currentWriteSize = m_socket->write(qExchange(m_header, {})); if (currentWriteSize != -1) currentWriteSize -= headerSize; + QMetaObject::invokeMethod(m_reply, "requestSent", Qt::QueuedConnection); } if (currentWriteSize == -1 || currentWriteSize != currentReadSize) { // socket broke down diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp index 649ec82f51..23c4ed8772 100644 --- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp @@ -9540,27 +9540,36 @@ void tst_QNetworkReply::moreActivitySignals_data() { QTest::addColumn("url"); QTest::addColumn("useipv6"); - QTest::addRow("local4") << QUrl("http://127.0.0.1") << false; - QTest::addRow("local6") << QUrl("http://[::1]") << true; + QTest::addColumn("postWithData"); + QTest::addRow("local4") << QUrl("http://127.0.0.1") << false << false; + QTest::addRow("local6") << QUrl("http://[::1]") << true << false; if (qEnvironmentVariable("QTEST_ENVIRONMENT").split(' ').contains("ci")) { // On CI server - QTest::addRow("localDns") << QUrl("http://localhost") << false; // will find v6 + QTest::addRow("localDns") << QUrl("http://localhost") << false << false; // will find v6 } else { // For manual testing - QTest::addRow("localDns4") << QUrl("http://localhost") << true; // will find both v4 and v6 - QTest::addRow("localDns6") << QUrl("http://localhost") << false; // will find both v4 and v6 + QTest::addRow("localDns4") << QUrl("http://localhost") << true << false; // will find both v4 and v6 + QTest::addRow("localDns6") << QUrl("http://localhost") << false << false; // will find both v4 and v6 } + QTest::addRow("post-with-data") << QUrl("http://[::1]") << true << true; } void tst_QNetworkReply::moreActivitySignals() { QFETCH(QUrl, url); QFETCH(bool, useipv6); + QFETCH(bool, postWithData); MiniHttpServer server(tst_QNetworkReply::httpEmpty200Response, false, nullptr/*thread*/, useipv6); server.doClose = false; url.setPort(server.serverPort()); QNetworkRequest request(url); - QNetworkReplyPtr reply(manager.get(request)); + QNetworkReplyPtr reply; + if (postWithData) { + request.setRawHeader("Content-Type", "text/plain"); + reply.reset(manager.post(request, "Hello, world!")); + } else { + reply.reset(manager.get(request)); + } QSignalSpy spy1(reply.data(), SIGNAL(socketStartedConnecting())); QSignalSpy spy2(reply.data(), SIGNAL(requestSent())); QSignalSpy spy3(reply.data(), SIGNAL(metaDataChanged())); @@ -9575,7 +9584,13 @@ void tst_QNetworkReply::moreActivitySignals() QCOMPARE(spy4.count(), 1); QVERIFY(reply->error() == QNetworkReply::NoError); // Second request will not send socketStartedConnecting because of keep-alive, so don't check it. - QNetworkReplyPtr secondreply(manager.get(request)); + QNetworkReplyPtr secondreply; + if (postWithData) { + request.setRawHeader("Content-Type", "text/plain"); + secondreply.reset(manager.post(request, "Hello, world!")); + } else { + secondreply.reset(manager.get(request)); + } QSignalSpy secondspy2(secondreply.data(), SIGNAL(requestSent())); QSignalSpy secondspy3(secondreply.data(), SIGNAL(metaDataChanged())); QSignalSpy secondspy4(secondreply.data(), SIGNAL(finished()));