Merge #786(kitsune): Another round of cleanup

.github/workflows/ci.yml

@@ -192,7 +192,7 @@ jobs:
         TEST_USER: ${{ secrets.TEST_USER }}
         TEST_PWD: ${{ secrets.TEST_PWD }}
         QT_ASSUME_STDERR_HAS_CONSOLE: 1 # Windows needs this for meaningful debug output
-        QT_LOGGING_RULES: 'quotient.main.debug=true;quotient.jobs.debug=true;quotient.events.debug=true'
+        QT_LOGGING_RULES: 'quotient.*.debug=true;quotient.jobs.sync.debug=false;quotient.events.ephemeral.debug=false;quotient.events.state.debug=false;quotient.events.members.debug=false'
         QT_MESSAGE_PATTERN: '%{time h:mm:ss.zzz}|%{category}|%{if-debug}D%{endif}%{if-info}I%{endif}%{if-warning}W%{endif}%{if-critical}C%{endif}%{if-fatal}F%{endif}|%{message}'
       run: |
         CTEST_ARGS="--test-dir $BUILD_PATH --output-on-failure"

Quotient/accountregistry.cpp

@@ -56,7 +56,7 @@ bool AccountRegistry::isLoggedIn(const QString &userId) const
 
 QVariant AccountRegistry::data(const QModelIndex& index, int role) const
 {
-    if (!index.isValid() || index.row() >= count())
+    if (!index.isValid() || index.row() >= size())
         return {};
 
     switch (role) {
@@ -71,7 +71,7 @@ QVariant AccountRegistry::data(const QModelIndex& index, int role) const
 
 int AccountRegistry::rowCount(const QModelIndex& parent) const
 {
-    return parent.isValid() ? 0 : count();
+    return parent.isValid() ? 0 : size();
 }
 
 QHash<int, QByteArray> AccountRegistry::roleNames() const

Quotient/connection.cpp

@@ -172,8 +172,7 @@ void Connection::loginWithToken(const QString& loginToken,
 
 void Connection::assumeIdentity(const QString& mxId, const QString& deviceId, const QString& accessToken)
 {
-    d->data->setDeviceId(deviceId);
-    d->completeSetup(mxId);
+    d->completeSetup(mxId, deviceId);
     d->ensureHomeserver(mxId).then([this, mxId, accessToken] {
         d->data->setToken(accessToken.toLatin1());
         callApi<GetTokenOwnerJob>().onResult([this, mxId](const GetTokenOwnerJob* job) {
@@ -289,24 +288,22 @@ void Connection::Private::dropAccessToken()
 template <typename... LoginArgTs>
 void Connection::Private::loginToServer(LoginArgTs&&... loginArgs)
 {
-    auto loginJob =
-            q->callApi<LoginJob>(std::forward<LoginArgTs>(loginArgs)...);
-    connect(loginJob, &BaseJob::success, q, [this, loginJob] {
-        data->setToken(loginJob->accessToken().toLatin1());
-        data->setDeviceId(loginJob->deviceId());
-        completeSetup(loginJob->userId());
-        saveAccessTokenToKeychain();
-        if (encryptionData)
-            encryptionData->database.clear();
-    });
-    connect(loginJob, &BaseJob::failure, q, [this, loginJob] {
-        emit q->loginError(loginJob->errorString(), loginJob->rawDataSample());
-    });
+    q->callApi<LoginJob>(std::forward<LoginArgTs>(loginArgs)...)
+        .onResult([this](const LoginJob* loginJob) {
+            if (loginJob->status().good()) {
+                data->setToken(loginJob->accessToken().toLatin1());
+                completeSetup(loginJob->userId(), loginJob->deviceId());
+                saveAccessTokenToKeychain();
+                if (encryptionData)
+                    encryptionData->database.clear();
+            } else
+                emit q->loginError(loginJob->errorString(), loginJob->rawDataSample());
+        });
 }
 
-void Connection::Private::completeSetup(const QString& mxId, bool mock)
+void Connection::Private::completeSetup(const QString& mxId, const QString& deviceId, bool mock)
 {
-    data->setUserId(mxId);
+    data->setIdentity(mxId, deviceId);
     q->setObjectName(data->userId() % u'/' % data->deviceId());
     qCDebug(MAIN) << "Using server" << data->baseUrl().toDisplayString()
                   << "by user" << data->userId()
@@ -800,7 +797,7 @@ JobHandle<CreateRoomJob> Connection::createRoom(
                                   creationContent, initialState, presetName, isDirect)
         .then(this, [this, invites, isDirect](const QString& roomId) {
             auto* room = provideRoom(roomId, JoinState::Join);
-            if (ALARM_X(!room, "Failed to create a room"))
+            if (QUO_ALARM_X(!room, "Failed to create a room object locally"))
                 return;
 
             emit createdRoom(room);
@@ -818,7 +815,7 @@ void Connection::requestDirectChat(const QString& userId)
 QFuture<Room*> Connection::getDirectChat(const QString& otherUserId)
 {
     auto* u = user(otherUserId);
-    if (ALARM_X(!u, u"Couldn't get a user object for" % otherUserId))
+    if (QUO_ALARM_X(!u, u"Couldn't get a user object for" % otherUserId))
         return {};
 
     // There can be more than one DC; find the first valid (existing and
@@ -1924,7 +1921,7 @@ Connection* Connection::makeMockConnection(const QString& mxId,
 {
     auto* c = new Connection;
     c->enableEncryption(enableEncryption);
-    c->d->completeSetup(mxId, true);
+    c->d->completeSetup(mxId, {}, true);
     return c;
 }
 

Quotient/connection_p.h

@@ -93,7 +93,7 @@ class Q_DECL_HIDDEN Quotient::Connection::Private {
     QFuture<void> ensureHomeserver(const QString& userId, const std::optional<LoginFlow>& flow = {});
     template <typename... LoginArgTs>
     void loginToServer(LoginArgTs&&... loginArgs);
-    void completeSetup(const QString &mxId, bool mock = false);
+    void completeSetup(const QString& mxId, const QString& deviceId, bool mock = false);
     void removeRoom(const QString& roomId);
 
     void consumeRoomData(SyncDataList&& roomDataList, bool fromCache);

Quotient/connectiondata.cpp

@@ -104,7 +104,7 @@ QUrl ConnectionData::baseUrl() const { return d->baseUrl; }
 
 HomeserverData ConnectionData::homeserverData() const
 {
-    return { d->baseUrl, d->supportedSpecVersions };
+    return { d->baseUrl, d->supportedSpecVersions, d->accessToken };
 }
 
 NetworkAccessManager* ConnectionData::nam() const
@@ -134,12 +134,11 @@ const QString& ConnectionData::deviceId() const { return d->deviceId; }
 
 const QString& ConnectionData::userId() const { return d->userId; }
 
-void ConnectionData::setDeviceId(const QString& deviceId)
-{
-    d->deviceId = deviceId;
-}
+void ConnectionData::setDeviceId(const QString& deviceId) { d->deviceId = deviceId; }
 
-void ConnectionData::setUserId(const QString& userId)
+void ConnectionData::setUserId(const QString& userId) { setIdentity(userId, d->deviceId); }
+
+void ConnectionData::setIdentity(const QString& userId, const QString& deviceId)
 {
     if (d->baseUrl.isValid()) {
         if (d->userId != userId)
@@ -150,13 +149,14 @@ void ConnectionData::setUserId(const QString& userId)
         }
     }
     d->userId = userId;
+    d->deviceId = deviceId;
 }
 
 void ConnectionData::setSupportedSpecVersions(QStringList versions)
 {
     qCInfo(MAIN).noquote() << "CS API versions:" << versions.join(u' ');
     d->supportedSpecVersions = std::move(versions);
-    if (!ALARM(d->userId.isEmpty()) && !ALARM(!d->baseUrl.isValid()))
+    if (QUO_CHECK(!d->userId.isEmpty()) && QUO_CHECK(d->baseUrl.isValid()))
         NetworkAccessManager::updateAccountSpecVersions(d->userId, d->supportedSpecVersions);
 }
 

Quotient/connectiondata.h

@@ -33,8 +33,11 @@ class QUOTIENT_API ConnectionData {
 
     void setBaseUrl(QUrl baseUrl);
     void setToken(QByteArray accessToken);
+    [[deprecated("Use setIdentity() instead")]]
     void setDeviceId(const QString& deviceId);
+    [[deprecated("Use setIdentity() instead")]]
     void setUserId(const QString& userId);
+    void setIdentity(const QString& userId, const QString& deviceId);
     void setSupportedSpecVersions(QStringList versions);
 
     QString lastEvent() const;

Quotient/e2ee/cryptoutils.cpp

@@ -65,16 +65,17 @@ inline std::pair<int, bool> checkedSize(
 
 // NOLINTBEGIN(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
 // TODO: remove NOLINT brackets once we're on clang-tidy 18
-#define CLAMP_SIZE(SizeVar_, ByteArray_, ...)                                                   \
-    const auto [SizeVar_, ByteArray_##Clamped] =                                                \
-        checkedSize((ByteArray_).size() __VA_OPT__(, ) __VA_ARGS__);                            \
-    if (ALARM_X(ByteArray_##Clamped,                                                            \
-                qPrintable(QStringLiteral(#ByteArray_ " is %1 bytes long, too much for OpenSSL" \
-                                                      " and overall suspicious")                \
-                               .arg((ByteArray_).size()))))                                     \
-        return SslPayloadTooLong;                                                               \
-    do {} while (false)                                                                         \
-    // End of macro
+#define CLAMP_SIZE(SizeVar_, ByteArray_, ...)                                             \
+    const auto [SizeVar_, ByteArray_##Clamped] =                                          \
+        checkedSize((ByteArray_).size() __VA_OPT__(, ) __VA_ARGS__);                      \
+    if (QUO_ALARM_X(ByteArray_##Clamped,                                                  \
+                    QStringLiteral(                                                       \
+                        #ByteArray_                                                       \
+                        " is %1 bytes long, too much for OpenSSL and overall suspicious") \
+                        .arg((ByteArray_).size())))                                       \
+        return SslPayloadTooLong;                                                         \
+    do {                                                                                  \
+    } while (false) // End of macro
 
 #define CALL_OPENSSL(Call_)                                                    \
     do {                                                                       \
@@ -106,7 +107,7 @@ SslExpected<QByteArray> Quotient::aesCtr256Encrypt(const QByteArray& plaintext,
     CLAMP_SIZE(plaintextSize, plaintext);
 
     const ContextHolder ctx(EVP_CIPHER_CTX_new(), &EVP_CIPHER_CTX_free);
-    if (ALARM_X(!ctx, QByteArrayLiteral("failed to create SSL context: ")
+    if (QUO_ALARM_X(!ctx, QByteArrayLiteral("failed to create SSL context: ")
                           + ERR_error_string(ERR_get_error(), nullptr)))
         return ERR_get_error();
 
@@ -311,7 +312,7 @@ std::vector<byte_t> Quotient::base58Decode(const QByteArray& encoded)
         }
     }
 
-    for (auto i = 0; i < encoded.length() && encoded[i] == '1'; ++i) {
+    for (auto i = 0; i < encoded.size() && encoded[i] == '1'; ++i) {
         result.push_back(0x0);
     }
 
@@ -323,7 +324,8 @@ QByteArray Quotient::sign(const QByteArray& key, const QByteArray& data)
 {
     auto context = makeCStruct(olm_pk_signing, olm_pk_signing_size, olm_clear_pk_signing);
     QByteArray pubKey(olm_pk_signing_public_key_length(), 0);
-    olm_pk_signing_key_from_seed(context.get(), pubKey.data(), pubKey.length(), key.data(), key.length());
+    olm_pk_signing_key_from_seed(context.get(), pubKey.data(), unsignedSize(pubKey), key.data(),
+                                 unsignedSize(key));
     Q_ASSERT(context);
 
     const auto signatureLength = olm_pk_signature_length();

Quotient/e2ee/e2ee_common.h

@@ -182,8 +182,6 @@ class QUOTIENT_API FixedBufferBase {
         return { std::bit_cast<const char*>(data_), untilPos };
     }
 
-    // TODO, 0.9: merge the overloads
-
     QByteArray toBase64() const { return viewAsByteArray().toBase64(); }
     QByteArray toBase64(QByteArray::Base64Options options) const
     {
@@ -218,11 +216,7 @@ class QUOTIENT_API FixedBuffer : public FixedBufferBase {
     static_assert(extent == std::dynamic_extent
                   || (extent < TotalSecureHeapSize / 2 && extent % 4 == 0));
 
-    FixedBuffer() // TODO, 0.9: merge with the next constructor
-        requires(extent != std::dynamic_extent)
-        : FixedBuffer(FillWithZeros)
-    {}
-    explicit FixedBuffer(InitOptions fillMode)
+    explicit FixedBuffer(InitOptions fillMode = FillWithZeros)
         requires(extent != std::dynamic_extent)
         : FixedBufferBase(ExtentN, fillMode)
     {}

Quotient/e2ee/qolmaccount.cpp

@@ -251,13 +251,12 @@ QOlmExpected<QOlmSession> QOlmAccount::createOutboundSession(
     const QByteArray& theirIdentityKey, const QByteArray& theirOneTimeKey) const
 {
     QOlmSession olmOutboundSession{};
-    if (const auto randomLength = olm_create_outbound_session_random_length(
-            olmOutboundSession.olmData);
-        olm_create_outbound_session(
-            olmOutboundSession.olmData, olmData, theirIdentityKey.data(),
-            theirIdentityKey.length(), theirOneTimeKey.data(),
-            theirOneTimeKey.length(), getRandom(randomLength).data(),
-            randomLength)
+    if (const auto randomLength =
+            olm_create_outbound_session_random_length(olmOutboundSession.olmData);
+        olm_create_outbound_session(olmOutboundSession.olmData, olmData, theirIdentityKey.data(),
+                                    unsignedSize(theirIdentityKey), theirOneTimeKey.data(),
+                                    unsignedSize(theirOneTimeKey), getRandom(randomLength).data(),
+                                    randomLength)
         == olm_error()) {
         const auto errorCode = olmOutboundSession.lastErrorCode();
         QOLM_FAIL_OR_LOG_X(errorCode == OLM_NOT_ENOUGH_RANDOM,

Quotient/jobs/basejob.cpp

@@ -9,6 +9,7 @@
 #include "../connectiondata.h"
 #include "../networkaccessmanager.h"
 
+#include <QtCore/QLibraryInfo>
 #include <QtCore/QMetaEnum>
 #include <QtCore/QPointer>
 #include <QtCore/QRegularExpression>
@@ -467,9 +468,9 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns)
             return true;
 
         auto patternParts = pattern.split('/');
-        if (ALARM_X(patternParts.size() > 2,
-                    "Expected content type should have up to two /-separated parts; violating pattern: "
-                        % pattern))
+        if (QUO_ALARM_X(patternParts.size() > 2, "Expected content type should have up to two "
+                                                 "parts separated by `/`; violating pattern: "
+                                                     % pattern))
             return false;
 
         if (ctype.split('/').front() == patternParts.front()
@@ -806,6 +807,8 @@ void BaseJob::abandon()
     if (d->reply)
         d->reply->disconnect(this);
     emit finished(this);
+    if (QLibraryInfo::version() < QVersionNumber(6, 5))
+        future().cancel(); // Qt 6.4 didn't do it on the promise destruction, see QTBUG-103992
 
     deleteLater(); // The promise will cancel itself on deletion
 }