From 5639a3424c628163e1672dc572c4ed0b347a8cf3 Mon Sep 17 00:00:00 2001 From: Andreas Loth Date: Thu, 9 Nov 2023 13:51:51 +0100 Subject: [PATCH] Axivion: Use signal/slot to start reading the network response Prevent use-after-free when QtCreator is closed while a network request is still on the way. The previous future approach will try to use reply despite reply being destroyed by networkAccessManager's destructor. Now, the connection gets automatically disconnected when reply gets destroyed. Change-Id: I7a00006a2f38d3a774eb2769e6faeb339d18025a Reviewed-by: hjk Reviewed-by: --- .../axivion/dashboard/dashboardclient.cpp | 110 ++++++++---------- 1 file changed, 50 insertions(+), 60 deletions(-) diff --git a/src/plugins/axivion/dashboard/dashboardclient.cpp b/src/plugins/axivion/dashboard/dashboardclient.cpp index 5137ad32461..3d20440b60a 100644 --- a/src/plugins/axivion/dashboard/dashboardclient.cpp +++ b/src/plugins/axivion/dashboard/dashboardclient.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include @@ -25,67 +26,47 @@ DashboardClient::DashboardClient(Utils::NetworkAccessManager &networkAccessManag { } -static void deleteLater(QObject *obj) -{ - obj->deleteLater(); -} - using ResponseData = Utils::expected, Error>; static constexpr int httpStatusCodeOk = 200; static const QLatin1String jsonContentType{ "application/json" }; -class ResponseReader final +ResponseData readResponse(QNetworkReply& reply, QAnyStringView expectedContentType) { -public: - ResponseReader(std::shared_ptr reply, QAnyStringView expectedContentType) - : m_reply(std::move(reply)), - m_expectedContentType(expectedContentType) - { } - - ~ResponseReader() { } - - ResponseData operator()() - { - QNetworkReply::NetworkError error = m_reply->error(); - int statusCode = m_reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); - QString contentType = m_reply->header(QNetworkRequest::ContentTypeHeader) - .toString() - .split(';') - .constFirst() - .trimmed() - .toLower(); - if (error == QNetworkReply::NetworkError::NoError - && statusCode == httpStatusCodeOk - && contentType == m_expectedContentType) { - return DataWithOrigin(m_reply->url(), m_reply->readAll()); - } - if (contentType == jsonContentType) { - try { - return tl::make_unexpected(DashboardError( - m_reply->url(), - statusCode, - m_reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(), - Dto::ErrorDto::deserialize(m_reply->readAll()))); - } catch (const Dto::invalid_dto_exception &) { - // ignore - } - } - if (statusCode != 0) { - return tl::make_unexpected(HttpError( - m_reply->url(), - statusCode, - m_reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(), - QString::fromUtf8(m_reply->readAll()))); // encoding? - } - return tl::make_unexpected( - NetworkError(m_reply->url(), error, m_reply->errorString())); + QNetworkReply::NetworkError error = reply.error(); + int statusCode = reply.attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); + QString contentType = reply.header(QNetworkRequest::ContentTypeHeader) + .toString() + .split(';') + .constFirst() + .trimmed() + .toLower(); + if (error == QNetworkReply::NetworkError::NoError + && statusCode == httpStatusCodeOk + && contentType == expectedContentType) { + return DataWithOrigin(reply.url(), reply.readAll()); } - -private: - std::shared_ptr m_reply; - QAnyStringView m_expectedContentType; -}; + if (contentType == jsonContentType) { + try { + return tl::make_unexpected(DashboardError( + reply.url(), + statusCode, + reply.attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(), + Dto::ErrorDto::deserialize(reply.readAll()))); + } catch (const Dto::invalid_dto_exception &) { + // ignore + } + } + if (statusCode != 0) { + return tl::make_unexpected(HttpError( + reply.url(), + statusCode, + reply.attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(), + QString::fromUtf8(reply.readAll()))); // encoding? + } + return tl::make_unexpected( + NetworkError(reply.url(), error, reply.errorString())); +} template static Utils::expected, Error> parseResponse(ResponseData rawBody) @@ -106,11 +87,13 @@ QFuture fetch(Utils::NetworkAccessManager &networkAccessManager, const std::optional &base, const QUrl &target) { + QPromise promise; + promise.start(); const AxivionServer &server = settings().server; QUrl url = base ? base->resolved(target) : target; QNetworkRequest request{ url }; request.setRawHeader(QByteArrayLiteral(u8"Accept"), - QByteArrayLiteral(u8"Application/Json")); + QByteArray(jsonContentType.data(), jsonContentType.size())); request.setRawHeader(QByteArrayLiteral(u8"Authorization"), QByteArrayLiteral(u8"AxToken ") + server.token.toUtf8()); QByteArray ua = QByteArrayLiteral(u8"Axivion") @@ -118,10 +101,17 @@ QFuture fetch(Utils::NetworkAccessManager &networkAccessManager, + QByteArrayLiteral(u8"Plugin/") + QCoreApplication::applicationVersion().toUtf8(); request.setRawHeader(QByteArrayLiteral(u8"X-Axivion-User-Agent"), ua); - std::shared_ptr reply{ networkAccessManager.get(request), deleteLater }; - return QtFuture::connect(reply.get(), &QNetworkReply::finished) - .onCanceled(reply.get(), [reply] { reply->abort(); }) - .then(ResponseReader(reply, jsonContentType)); + QFuture future = promise.future(); + QNetworkReply* reply = networkAccessManager.get(request); + QObject::connect(reply, + &QNetworkReply::finished, + reply, + [promise = std::move(promise), reply]() mutable { + promise.addResult(readResponse(*reply, jsonContentType)); + promise.finish(); + reply->deleteLater(); + }); + return future; } QFuture DashboardClient::fetchProjectInfo(const QString &projectName) @@ -137,4 +127,4 @@ QFuture DashboardClient::fetchProjectInfo(const .then(QtFuture::Launch::Async, &parseResponse); } -} +} // namespace Axivion::Internal