diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp index c805f8e8adc..b116d26805a 100644 --- a/src/network/access/qhttpnetworkconnectionchannel.cpp +++ b/src/network/access/qhttpnetworkconnectionchannel.cpp @@ -271,7 +271,7 @@ void QHttpNetworkConnectionChannel::_q_readyRead() void QHttpNetworkConnectionChannel::handleUnexpectedEOF() { Q_ASSERT(reply); - if (reconnectAttempts <= 0) { + if (reconnectAttempts <= 0 || !request.methodIsIdempotent()) { // too many errors reading/receiving/parsing the status, close the socket and emit error requeueCurrentlyPipelinedRequests(); close(); diff --git a/src/network/access/qhttpnetworkrequest.cpp b/src/network/access/qhttpnetworkrequest.cpp index 23a3972638a..03af2573e54 100644 --- a/src/network/access/qhttpnetworkrequest.cpp +++ b/src/network/access/qhttpnetworkrequest.cpp @@ -394,5 +394,13 @@ void QHttpNetworkRequest::setFullLocalServerName(const QString &fullServerName) d->fullLocalServerName = fullServerName; } +bool QHttpNetworkRequest::methodIsIdempotent() const +{ + using Op = Operation; + constexpr auto knownSafe = std::array{ Op::Get, Op::Head, Op::Put, Op::Trace, Op::Options }; + return std::any_of(knownSafe.begin(), knownSafe.end(), + [currentOp = d->operation](auto op) { return op == currentOp; }); +} + QT_END_NAMESPACE diff --git a/src/network/access/qhttpnetworkrequest_p.h b/src/network/access/qhttpnetworkrequest_p.h index 0b8941c8f32..8cbed9a761c 100644 --- a/src/network/access/qhttpnetworkrequest_p.h +++ b/src/network/access/qhttpnetworkrequest_p.h @@ -120,6 +120,8 @@ public: QString fullLocalServerName() const; void setFullLocalServerName(const QString &fullServerName); + bool methodIsIdempotent() const; + private: QSharedDataPointer d; friend class QHttpNetworkRequestPrivate; diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp index a3a8aaad7f5..d93ee07209c 100644 --- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp @@ -600,6 +600,9 @@ private Q_SLOTS: void abortAndError(); + void resendRequest_data(); + void resendRequest(); + // NOTE: This test must be last! void parentingRepliesToTheApp(); private: @@ -678,6 +681,7 @@ public: QByteArray receivedData; QSemaphore ready; bool doClose; + bool earlyClose = false; // close connection after request has been received bool doSsl; bool ipv6; bool multiple; @@ -805,6 +809,9 @@ private slots: qDebug() << "slotError" << err << currentClient->errorString(); } +signals: + void requestReceived() const; + public slots: void readyReadSlot() @@ -830,10 +837,16 @@ public slots: if (contentRead < contentLength) return; + emit requestReceived(); + // multiple requests incoming. remove the bytes of the current one if (multiple) receivedData.remove(0, endOfHeader); + if (earlyClose) { + client->disconnectFromHost(); + return; + } reply(); } } @@ -10740,6 +10753,56 @@ Hello World!)"_ba; QCOMPARE(reply->error(), QNetworkReply::OperationCanceledError); } +void tst_QNetworkReply::resendRequest_data(){ + QTest::addColumn("method"); + QTest::addColumn("shouldResend"); + + for (auto &method : { "get", "head", "put" }) + QTest::addRow("%s", method) << method << true; + QTest::addRow("post") << "post" << false; + QTest::addRow("mycustom") << "mycustom" << false; + +} + +void tst_QNetworkReply::resendRequest() +{ + QFETCH(const QString, method); + QFETCH(const bool, shouldResend); + + MiniHttpServer server(""); + server.earlyClose = true; + + QSignalSpy requestReceived(&server, &MiniHttpServer::requestReceived); + + QUrl url("http://127.0.0.1"); + url.setPort(server.serverPort()); + const QByteArray data(4096, 'a'); + QNetworkReplyPtr reply([&]() { + QNetworkRequest req(url); + if (method == "get") + return manager.get(req, data); + else if (method == "head") + return manager.head(req); + else if (method == "put") + return manager.put(req, data); + else + return manager.sendCustomRequest(req, method.toUtf8(), data); + }()); + + // We send one request and will get no response from the server: + QVERIFY(requestReceived.wait()); + requestReceived.clear(); + // Then, for idempotent requests, we send the request again. For + // non-idempotent requests we error out and don't try to resend. + QCOMPARE(requestReceived.wait(2s), shouldResend); + if (!shouldResend) { + QCOMPARE(reply->error(), QNetworkReply::RemoteHostClosedError); + } else { + // No error yet, still can resend another + QCOMPARE(reply->error(), QNetworkReply::NoError); + } +} + // NOTE: This test must be last testcase in tst_qnetworkreply! void tst_QNetworkReply::parentingRepliesToTheApp() {