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 <hjk@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
This commit is contained in:
Andreas Loth
2023-11-09 13:51:51 +01:00
parent aabf03ec06
commit 5639a3424c

View File

@@ -14,6 +14,7 @@
#include <QLatin1String> #include <QLatin1String>
#include <QNetworkReply> #include <QNetworkReply>
#include <QNetworkRequest> #include <QNetworkRequest>
#include <QPromise>
#include <memory> #include <memory>
@@ -25,31 +26,16 @@ DashboardClient::DashboardClient(Utils::NetworkAccessManager &networkAccessManag
{ {
} }
static void deleteLater(QObject *obj)
{
obj->deleteLater();
}
using ResponseData = Utils::expected<DataWithOrigin<QByteArray>, Error>; using ResponseData = Utils::expected<DataWithOrigin<QByteArray>, Error>;
static constexpr int httpStatusCodeOk = 200; static constexpr int httpStatusCodeOk = 200;
static const QLatin1String jsonContentType{ "application/json" }; static const QLatin1String jsonContentType{ "application/json" };
class ResponseReader final ResponseData readResponse(QNetworkReply& reply, QAnyStringView expectedContentType)
{ {
public: QNetworkReply::NetworkError error = reply.error();
ResponseReader(std::shared_ptr<QNetworkReply> reply, QAnyStringView expectedContentType) int statusCode = reply.attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
: m_reply(std::move(reply)), QString contentType = reply.header(QNetworkRequest::ContentTypeHeader)
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() .toString()
.split(';') .split(';')
.constFirst() .constFirst()
@@ -57,36 +43,31 @@ public:
.toLower(); .toLower();
if (error == QNetworkReply::NetworkError::NoError if (error == QNetworkReply::NetworkError::NoError
&& statusCode == httpStatusCodeOk && statusCode == httpStatusCodeOk
&& contentType == m_expectedContentType) { && contentType == expectedContentType) {
return DataWithOrigin(m_reply->url(), m_reply->readAll()); return DataWithOrigin(reply.url(), reply.readAll());
} }
if (contentType == jsonContentType) { if (contentType == jsonContentType) {
try { try {
return tl::make_unexpected(DashboardError( return tl::make_unexpected(DashboardError(
m_reply->url(), reply.url(),
statusCode, statusCode,
m_reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(), reply.attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(),
Dto::ErrorDto::deserialize(m_reply->readAll()))); Dto::ErrorDto::deserialize(reply.readAll())));
} catch (const Dto::invalid_dto_exception &) { } catch (const Dto::invalid_dto_exception &) {
// ignore // ignore
} }
} }
if (statusCode != 0) { if (statusCode != 0) {
return tl::make_unexpected(HttpError( return tl::make_unexpected(HttpError(
m_reply->url(), reply.url(),
statusCode, statusCode,
m_reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(), reply.attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(),
QString::fromUtf8(m_reply->readAll()))); // encoding? QString::fromUtf8(reply.readAll()))); // encoding?
} }
return tl::make_unexpected( return tl::make_unexpected(
NetworkError(m_reply->url(), error, m_reply->errorString())); NetworkError(reply.url(), error, reply.errorString()));
} }
private:
std::shared_ptr<QNetworkReply> m_reply;
QAnyStringView m_expectedContentType;
};
template<typename T> template<typename T>
static Utils::expected<DataWithOrigin<T>, Error> parseResponse(ResponseData rawBody) static Utils::expected<DataWithOrigin<T>, Error> parseResponse(ResponseData rawBody)
{ {
@@ -106,11 +87,13 @@ QFuture<ResponseData> fetch(Utils::NetworkAccessManager &networkAccessManager,
const std::optional<QUrl> &base, const std::optional<QUrl> &base,
const QUrl &target) const QUrl &target)
{ {
QPromise<ResponseData> promise;
promise.start();
const AxivionServer &server = settings().server; const AxivionServer &server = settings().server;
QUrl url = base ? base->resolved(target) : target; QUrl url = base ? base->resolved(target) : target;
QNetworkRequest request{ url }; QNetworkRequest request{ url };
request.setRawHeader(QByteArrayLiteral(u8"Accept"), request.setRawHeader(QByteArrayLiteral(u8"Accept"),
QByteArrayLiteral(u8"Application/Json")); QByteArray(jsonContentType.data(), jsonContentType.size()));
request.setRawHeader(QByteArrayLiteral(u8"Authorization"), request.setRawHeader(QByteArrayLiteral(u8"Authorization"),
QByteArrayLiteral(u8"AxToken ") + server.token.toUtf8()); QByteArrayLiteral(u8"AxToken ") + server.token.toUtf8());
QByteArray ua = QByteArrayLiteral(u8"Axivion") QByteArray ua = QByteArrayLiteral(u8"Axivion")
@@ -118,10 +101,17 @@ QFuture<ResponseData> fetch(Utils::NetworkAccessManager &networkAccessManager,
+ QByteArrayLiteral(u8"Plugin/") + QByteArrayLiteral(u8"Plugin/")
+ QCoreApplication::applicationVersion().toUtf8(); + QCoreApplication::applicationVersion().toUtf8();
request.setRawHeader(QByteArrayLiteral(u8"X-Axivion-User-Agent"), ua); request.setRawHeader(QByteArrayLiteral(u8"X-Axivion-User-Agent"), ua);
std::shared_ptr<QNetworkReply> reply{ networkAccessManager.get(request), deleteLater }; QFuture<ResponseData> future = promise.future();
return QtFuture::connect(reply.get(), &QNetworkReply::finished) QNetworkReply* reply = networkAccessManager.get(request);
.onCanceled(reply.get(), [reply] { reply->abort(); }) QObject::connect(reply,
.then(ResponseReader(reply, jsonContentType)); &QNetworkReply::finished,
reply,
[promise = std::move(promise), reply]() mutable {
promise.addResult(readResponse(*reply, jsonContentType));
promise.finish();
reply->deleteLater();
});
return future;
} }
QFuture<DashboardClient::RawProjectInfo> DashboardClient::fetchProjectInfo(const QString &projectName) QFuture<DashboardClient::RawProjectInfo> DashboardClient::fetchProjectInfo(const QString &projectName)
@@ -137,4 +127,4 @@ QFuture<DashboardClient::RawProjectInfo> DashboardClient::fetchProjectInfo(const
.then(QtFuture::Launch::Async, &parseResponse<Dto::ProjectInfoDto>); .then(QtFuture::Launch::Async, &parseResponse<Dto::ProjectInfoDto>);
} }
} } // namespace Axivion::Internal