Http: fix issues after early abort()

There were a few issues to deal with. One of them was that we would
print a warning about an internal error if we tried to *set* an error
on the reply after it had been cancelled. That's kind of unavoidable
since these things happen in different threads, so just ignore the
error if we have been cancelled.

The other issue was that, for a request with data, we will buffer the
data to send, and _only then_ do we start the request. This happens
asynchronously, so the user can abort the request before it has finished
buffering. Once it finished buffering it would set the state of the
request to "Working", ignoring that it was already marked "Finished".

Fixes: QTBUG-118209
Fixes: QTBUG-36127
Pick-to: 6.6
Change-Id: Idbf1fd8a80530d802bee04c4b0a6783cba4992d3
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit be6644c1f254cce796c43d7c1eae3ae794bb77be)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Mårten Nordheim 2024-03-05 14:36:09 +01:00 committed by Qt Cherry-pick Bot
parent 3789e5972e
commit 518ce19c75
2 changed files with 41 additions and 2 deletions

View File

@ -1888,7 +1888,9 @@ void QNetworkReplyHttpImplPrivate::setResumeOffset(quint64 offset)
void QNetworkReplyHttpImplPrivate::_q_startOperation()
{
if (state == Working) // ensure this function is only being called once
// Ensure this function is only being called once, and not at all if we were
// cancelled
if (state >= Working)
return;
state = Working;
@ -2156,7 +2158,9 @@ void QNetworkReplyHttpImplPrivate::error(QNetworkReplyImpl::NetworkError code, c
Q_Q(QNetworkReplyHttpImpl);
// Can't set and emit multiple errors.
if (errorCode != QNetworkReply::NoError) {
qWarning("QNetworkReplyImplPrivate::error: Internal problem, this method must only be called once.");
// But somewhat unavoidable if we have cancelled the request:
if (errorCode != QNetworkReply::OperationCanceledError)
qWarning("QNetworkReplyImplPrivate::error: Internal problem, this method must only be called once.");
return;
}

View File

@ -559,6 +559,8 @@ private Q_SLOTS:
void qtbug68821proxyError_data();
void qtbug68821proxyError();
void abortAndError();
// NOTE: This test must be last!
void parentingRepliesToTheApp();
private:
@ -10417,6 +10419,39 @@ void tst_QNetworkReply::qtbug68821proxyError()
QCOMPARE(spy.at(0).at(0), error);
}
void tst_QNetworkReply::abortAndError()
{
const QByteArray response =
R"(HTTP/1.0 500 Internal Server Error
Content-Length: 12
Content-Type: text/plain
Hello World!)"_ba;
MiniHttpServer server(response);
QNetworkAccessManager manager;
QNetworkRequest req(QUrl("http://127.0.0.1:" + QString::number(server.serverPort())));
std::unique_ptr<QNetworkReply> reply(manager.post(req, "my data goes here"_ba));
QSignalSpy errorSignal(reply.get(), &QNetworkReply::errorOccurred);
QSignalSpy finishedSignal(reply.get(), &QNetworkReply::finished);
reply->abort();
// We don't want to print this warning in this case because it is impossible
// for users to avoid it.
QTest::failOnWarning("QNetworkReplyImplPrivate::error: Internal problem, this method must only "
"be called once.");
// Process any signals from the http thread:
QTest::qWait(1s);
if (QTest::currentTestFailed())
return;
QCOMPARE(finishedSignal.count(), 1);
QCOMPARE(errorSignal.count(), 1);
QCOMPARE(reply->error(), QNetworkReply::OperationCanceledError);
}
// NOTE: This test must be last testcase in tst_qnetworkreply!
void tst_QNetworkReply::parentingRepliesToTheApp()
{