From 7c0742aa2b0b548bf99abe43c1a0410a11576893 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 25 Jan 2021 21:33:40 +0100 Subject: Connection: stop login flows job before resolving This avoids a corner case when a login flows job finishes (or worse, goes for a retry) while the homeserver is (being) resolved, yielding the Connection object in an inconsistent state to the client. --- lib/connection.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index 3ed71bb4..0fb2d003 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -133,6 +133,11 @@ public: != "json"; bool lazyLoading = false; + /// \brief Stop resolving and login flows jobs, and clear login flows + /// + /// Prepares the class to set or resolve a new homeserver + void clearResolvingContext(); + /** \brief Check the homeserver and resolve it if needed, before connecting * * A single entry for functions that need to check whether the homeserver @@ -270,8 +275,7 @@ Connection::~Connection() void Connection::resolveServer(const QString& mxid) { - if (isJobRunning(d->resolverJob)) - d->resolverJob->abandon(); + d->clearResolvingContext(); auto maybeBaseUrl = QUrl::fromUserInput(serverPart(mxid)); maybeBaseUrl.setScheme("https"); // Instead of the Qt-default "http" @@ -1530,13 +1534,19 @@ QByteArray Connection::generateTxnId() const return d->data->generateTxnId(); } +void Connection::Private::clearResolvingContext() +{ + if (isJobRunning(resolverJob)) + resolverJob->abandon(); + if (isJobRunning(loginFlowsJob)) + loginFlowsJob->abandon(); + loginFlows.clear(); + +} + void Connection::setHomeserver(const QUrl& url) { - if (isJobRunning(d->resolverJob)) - d->resolverJob->abandon(); - if (isJobRunning(d->loginFlowsJob)) - d->loginFlowsJob->abandon(); - d->loginFlows.clear(); + d->clearResolvingContext(); if (homeserver() != url) { d->data->setBaseUrl(url); -- cgit v1.2.3 From b069c6bb377641840026aa37cb8acf7e2a76bfe5 Mon Sep 17 00:00:00 2001 From: Roland Pallai Date: Thu, 28 Jan 2021 14:58:11 +0100 Subject: Fix rich replies json format (transmit) With this patch it looks like: "m.relates_to": { "m.in_reply_to": { "event_id": "$another:event.com" } } instead of: "m.relates_to": { "event_id": "$another:event.com", "rel_type": "m.in_reply_to" }, So it fits the specification by now. https://matrix.org/docs/spec/client_server/r0.6.1#rich-replies (cherry picked from commit b850edadde2299b122a5cd17da85e943430e43b7) --- lib/events/roommessageevent.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/events/roommessageevent.cpp b/lib/events/roommessageevent.cpp index 616a034f..e2465e61 100644 --- a/lib/events/roommessageevent.cpp +++ b/lib/events/roommessageevent.cpp @@ -338,7 +338,10 @@ void TextContent::fillJson(QJsonObject* json) const } if (relatesTo) { json->insert(QStringLiteral("m.relates_to"), - QJsonObject { { "rel_type", relatesTo->type }, { EventIdKey, relatesTo->eventId } }); + relatesTo->type == RelatesTo::ReplyTypeId() ? + QJsonObject { { relatesTo->type, QJsonObject{ { EventIdKey, relatesTo->eventId } } } } : + QJsonObject { { "rel_type", relatesTo->type }, { EventIdKey, relatesTo->eventId } } + ); if (relatesTo->type == RelatesTo::ReplacementTypeId()) { QJsonObject newContentJson; if (mimeType.inherits("text/html")) { -- cgit v1.2.3 From 221cc9faf713eb383a0e76a56018794796690369 Mon Sep 17 00:00:00 2001 From: Roland Pallai Date: Thu, 28 Jan 2021 15:05:33 +0100 Subject: Fix rich edits (transmit) The new formatted_body was not included into new content on edit due to badly constructed json. (cherry picked from commit df6b2d31ec8f2f5890826719e960f450a4968f22) --- lib/events/roommessageevent.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/events/roommessageevent.cpp b/lib/events/roommessageevent.cpp index e2465e61..e68e7d63 100644 --- a/lib/events/roommessageevent.cpp +++ b/lib/events/roommessageevent.cpp @@ -345,8 +345,8 @@ void TextContent::fillJson(QJsonObject* json) const if (relatesTo->type == RelatesTo::ReplacementTypeId()) { QJsonObject newContentJson; if (mimeType.inherits("text/html")) { - json->insert(FormatKey, HtmlContentTypeId); - json->insert(FormattedBodyKey, body); + newContentJson.insert(FormatKey, HtmlContentTypeId); + newContentJson.insert(FormattedBodyKey, body); } json->insert(QStringLiteral("m.new_content"), newContentJson); } -- cgit v1.2.3 From 86f24d1ecf300b82b3a7253b81a2c392669d2c2b Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 21 Feb 2021 20:20:38 +0100 Subject: Uri: support abbreviated types in Matrix URIs As per the latest iteration of MSC2312, room/, user/ and event/ are only supported for parsing and replication but not for emitting from Matrix identifiers. --- lib/uri.cpp | 15 +++++++++++---- tests/quotest.cpp | 4 ++++ 2 files changed, 15 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/uri.cpp b/lib/uri.cpp index e0912eb6..9eefdc83 100644 --- a/lib/uri.cpp +++ b/lib/uri.cpp @@ -7,13 +7,19 @@ using namespace Quotient; struct ReplacePair { QByteArray uriString; char sigil; }; -/// Defines bi-directional mapping of path prefixes and sigils +/// \brief Defines bi-directional mapping of path prefixes and sigils +/// +/// When there are two prefixes for the same sigil, the first matching +/// entry for a given sigil is used. static const auto replacePairs = { - ReplacePair { "user/", '@' }, + ReplacePair { "u/", '@' }, + { "user/", '@' }, { "roomid/", '!' }, + { "r/", '#' }, { "room/", '#' }, // The notation for bare event ids is not proposed in MSC2312 but there's // https://github.com/matrix-org/matrix-doc/pull/2644 + { "e/", '$' }, { "event/", '$' } }; @@ -94,7 +100,7 @@ Uri::Uri(QUrl url) : QUrl(std::move(url)) case 2: break; case 4: - if (splitPath[2] == "event") + if (splitPath[2] == "event" || splitPath[2] == "e") break; [[fallthrough]]; default: @@ -147,7 +153,8 @@ Uri::Type Uri::type() const { return primaryType_; } Uri::SecondaryType Uri::secondaryType() const { - return pathSegment(*this, 2) == "event" ? EventId : NoSecondaryId; + const auto& type2 = pathSegment(*this, 2); + return type2 == "event" || type2 == "e" ? EventId : NoSecondaryId; } QUrl Uri::toUrl(UriForm form) const diff --git a/tests/quotest.cpp b/tests/quotest.cpp index bcc5a119..6ad5e8a8 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -726,12 +726,15 @@ TEST_IMPL(visitResources) roomId, "matrix:roomid/" + roomId.mid(1), "https://matrix.to/#/%21"/*`!`*/ + roomId.mid(1), roomAlias, "matrix:room/" + roomAlias.mid(1), + "matrix:r/" + roomAlias.mid(1), "https://matrix.to/#/" + roomAlias, }; const QStringList userUris { userId, "matrix:user/" + userId.mid(1), + "matrix:u/" + userId.mid(1), "https://matrix.to/#/" + userId }; const QStringList eventUris { "matrix:room/" + roomAlias.mid(1) + "/event/" + eventId.mid(1), + "matrix:r/" + roomAlias.mid(1) + "/e/" + eventId.mid(1), "https://matrix.to/#/" + roomId + '/' + eventId }; // Check that reserved characters are correctly processed. @@ -745,6 +748,7 @@ TEST_IMPL(visitResources) static const QStringList joinByAliasUris { Uri(joinRoomAlias.toUtf8(), {}, joinQuery.mid(1)).toDisplayString(), "matrix:room/" + encodedRoomAliasNoSigil + joinQuery, + "matrix:r/" + encodedRoomAliasNoSigil + joinQuery, "https://matrix.to/#/%23"/*`#`*/ + encodedRoomAliasNoSigil + joinQuery, "https://matrix.to/#/%23" + joinRoomAlias.mid(1) /* unencoded */ + joinQuery }; -- cgit v1.2.3 From b25785d294669f2bab7dcd1e3cd1fba61991fe46 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 21 Feb 2021 20:47:36 +0100 Subject: Update a comment that still mentions Riot --- lib/events/roommessageevent.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/events/roommessageevent.cpp b/lib/events/roommessageevent.cpp index e68e7d63..de499e7c 100644 --- a/lib/events/roommessageevent.cpp +++ b/lib/events/roommessageevent.cpp @@ -313,7 +313,7 @@ TextContent::TextContent(const QJsonObject& json) const auto actualJson = isReplacement(relatesTo) ? json.value("m.new_content"_ls).toObject() : json; - // Special-casing the custom matrix.org's (actually, Riot's) way + // Special-casing the custom matrix.org's (actually, Element's) way // of sending HTML messages. if (actualJson["format"_ls].toString() == HtmlContentTypeId) { mimeType = HtmlMimeType; -- cgit v1.2.3 From 6dc8bc05a95324de512ef7999b47507e712ccb76 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Thu, 4 Mar 2021 14:25:21 +0100 Subject: Tighten up against malformed user ids in events A few months ago 3c85f049 introduced validation of user ids but the rest of the library code wasn't updated to the fact that Connection::user() may quite legitimately (if not routinely) return nullptr, leading to crashes particularaly when malformed ids come from the wire. This commit adds necessary checks before using the value returned from user(). Closes #456. --- lib/connection.cpp | 10 ++++++++-- lib/room.cpp | 37 ++++++++++++++++++++++++------------- lib/uriresolver.cpp | 6 +++--- 3 files changed, 35 insertions(+), 18 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index 0fb2d003..c4587f30 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -691,7 +691,7 @@ void Connection::Private::consumeAccountData(Events&& accountDataEvents) DirectChatsMap remoteAdditions; for (auto it = usersToDCs.begin(); it != usersToDCs.end(); ++it) { - if (auto* u = q->user(it.key())) { + if (auto* const u = q->user(it.key())) { if (!directChats.contains(u, it.value()) && !dcLocalRemovals.contains(u, it.value())) { Q_ASSERT(!directChatUsers.contains(it.value(), u)); @@ -924,6 +924,12 @@ Connection::createRoom(RoomVisibility visibility, const QString& alias, const QJsonObject& creationContent) { invites.removeOne(userId()); // The creator is by definition in the room + for (const auto& i : invites) + if (!user(i)) { + qCWarning(MAIN) << "Won't create a room with malformed invitee ids"; + return nullptr; + } + auto job = callApi(visibility == PublishRoom ? QStringLiteral("public") : QStringLiteral("private"), @@ -958,7 +964,7 @@ void Connection::requestDirectChat(User* u) void Connection::doInDirectChat(const QString& userId, const std::function& operation) { - if (auto* u = user(userId)) + if (auto* const u = user(userId)) doInDirectChat(u, operation); else qCCritical(MAIN) diff --git a/lib/room.cpp b/lib/room.cpp index c42e618e..a5940eb2 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1460,7 +1460,9 @@ QString Room::roomMembername(const User* u) const QString Room::roomMembername(const QString& userId) const { - return roomMembername(user(userId)); + if (auto* const u = user(userId)) + return roomMembername(u); + return {}; } QString Room::safeMemberName(const QString& userId) const @@ -2349,7 +2351,7 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events) // the new message events. if (const auto senderId = (*from)->senderId(); !senderId.isEmpty()) { auto* const firstWriter = q->user(senderId); - if (q->readMarker(firstWriter) != timeline.crend()) { + if (firstWriter && q->readMarker(firstWriter) != timeline.crend()) { roomChanges |= promoteReadMarker(firstWriter, rev_iter_t(from) - 1); qCDebug(MESSAGES) @@ -2418,6 +2420,13 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) if (!e.isStateEvent()) return Change::NoChange; + auto* const sender = user(e.senderId()); + if (!sender) { + qCWarning(MAIN) << "State event" << e.id() + << "is invalid and won't be processed"; + return Change::NoChange; + } + // Find a value (create an empty one if necessary) and get a reference // to it. Can't use getCurrentState<>() because it (creates and) returns // a stub if a value is not found, and what's needed here is a "real" event @@ -2426,9 +2435,9 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) // Prepare for the state change const auto oldRme = static_cast(curStateEvent); visit(e, [this, &oldRme](const RoomMemberEvent& rme) { - auto* u = user(rme.userId()); - if (!u) { // ??? - qCCritical(MAIN) + auto* const u = user(rme.userId()); + if (!u) { // Invalid user id? + qCWarning(MAIN) << "Could not get a user object for" << rme.userId(); return; } @@ -2517,9 +2526,11 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) emit avatarChanged(); return AvatarChange; } - , [this,oldRme] (const RoomMemberEvent& evt) { + , [this,oldRme,sender] (const RoomMemberEvent& evt) { // clang-format on auto* u = user(evt.userId()); + if (!u) + return NoChange; // Already warned earlier // TODO: remove in 0.7 u->processEvent(evt, this, oldRme == nullptr); @@ -2539,7 +2550,7 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) if (!d->usersInvited.contains(u)) d->usersInvited.push_back(u); if (u == localUser() && evt.isDirect()) - connection()->addToDirectChats(this, user(evt.senderId())); + connection()->addToDirectChats(this, sender); break; case MembershipType::Knock: case MembershipType::Ban: @@ -2599,8 +2610,8 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event) if (auto* evt = eventCast(event)) { d->usersTyping.clear(); for (const QString& userId : qAsConst(evt->users())) { - auto u = user(userId); - if (memberJoinState(u) == JoinState::Join) + auto* const u = user(userId); + if (u && memberJoinState(u) == JoinState::Join) d->usersTyping.append(u); } if (evt->users().size() > 3 || et.nsecsElapsed() >= profilerMinNsecs()) @@ -2625,8 +2636,8 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event) for (const Receipt& r : p.receipts) { if (r.userId == connection()->userId()) continue; // FIXME, #185 - auto u = user(r.userId); - if (memberJoinState(u) == JoinState::Join) + auto* const u = user(r.userId); + if (u && memberJoinState(u) == JoinState::Join) changes |= d->promoteReadMarker(u, newMarker); } } else { @@ -2639,8 +2650,8 @@ Room::Changes Room::processEphemeralEvent(EventPtr&& event) for (const Receipt& r : p.receipts) { if (r.userId == connection()->userId()) continue; // FIXME, #185 - auto u = user(r.userId); - if (memberJoinState(u) == JoinState::Join + auto* const u = user(r.userId); + if (u && memberJoinState(u) == JoinState::Join && readMarker(u) == timelineEdge()) changes |= d->setLastReadEvent(u, p.evtId); } diff --git a/lib/uriresolver.cpp b/lib/uriresolver.cpp index 27360bcc..e5f19a96 100644 --- a/lib/uriresolver.cpp +++ b/lib/uriresolver.cpp @@ -24,9 +24,9 @@ UriResolveResult UriResolverBase::visitResource(Connection* account, case Uri::UserId: { if (uri.action() == "join") return IncorrectAction; - auto* user = account->user(uri.primaryId()); - Q_ASSERT(user != nullptr); - return visitUser(user, uri.action()); + if (auto* const user = account->user(uri.primaryId())) + return visitUser(user, uri.action()); + return InvalidUri; } case Uri::RoomId: case Uri::RoomAlias: { -- cgit v1.2.3