From 025e5ab7d90ce8cf474567457301de32d5a3e34a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 6 Jul 2019 20:13:38 +0900 Subject: Be stricter on usage of stateKey A few places in the library dealt with state events without any notion of state_key inside events, including StateEvent[Base] and relevant functions in Room. A number of workarounds have been made; e.g., Room::setMemberState() accepted userId as a separate parameter, ignoring the state key inside the RoomMemberEvent already passed to it, and Room::setLocalAliases() had a bug in the initial version where the function still tried to pass aliases in an event with an empty state key. This commit fixes this shortcoming: StateEventBase now gets stateKey as one more parameter, Room::Private::getCurrentState() respects stateKey and returns properly constructed stub events, and Room::setMemberState() gives way to a more generic Room::setState() that works uniformly with whatever state event you pass to it. --- lib/events/roommemberevent.h | 8 +++++- lib/events/simplestateevents.h | 24 ++++++++++++----- lib/events/stateevent.cpp | 6 +++++ lib/events/stateevent.h | 10 +++++-- lib/room.cpp | 60 ++++++++++++++++++++++++++++-------------- lib/room.h | 4 +++ lib/user.cpp | 2 +- 7 files changed, 84 insertions(+), 30 deletions(-) diff --git a/lib/events/roommemberevent.h b/lib/events/roommemberevent.h index 4490fe65..39aa280c 100644 --- a/lib/events/roommemberevent.h +++ b/lib/events/roommemberevent.h @@ -56,8 +56,14 @@ namespace QMatrixClient explicit RoomMemberEvent(const QJsonObject& obj) : StateEvent(typeId(), obj) { } + [[deprecated("Use RoomMemberEvent(userId, contentArgs) instead")]] RoomMemberEvent(MemberEventContent&& c) - : StateEvent(typeId(), matrixTypeId(), c) + : StateEvent(typeId(), matrixTypeId(), QString(), c) + { } + template + RoomMemberEvent(const QString& userId, ArgTs&&... contentArgs) + : StateEvent(typeId(), matrixTypeId(), userId, + std::forward(contentArgs)...) { } /// A special constructor to create unknown RoomMemberEvents diff --git a/lib/events/simplestateevents.h b/lib/events/simplestateevents.h index 2c23d9ca..dc6a0868 100644 --- a/lib/events/simplestateevents.h +++ b/lib/events/simplestateevents.h @@ -20,8 +20,6 @@ #include "stateevent.h" -#include "converters.h" - namespace QMatrixClient { namespace EventContent @@ -63,7 +61,7 @@ namespace QMatrixClient explicit _Name() : _Name(value_type()) { } \ template \ explicit _Name(T&& value) \ - : StateEvent(typeId(), matrixTypeId(), \ + : StateEvent(typeId(), matrixTypeId(), QString(), \ QStringLiteral(#_ContentKey), \ std::forward(value)) \ { } \ @@ -78,9 +76,6 @@ namespace QMatrixClient DEFINE_SIMPLE_STATE_EVENT(RoomNameEvent, "m.room.name", QString, name) DEFINE_EVENTTYPE_ALIAS(RoomName, RoomNameEvent) - DEFINE_SIMPLE_STATE_EVENT(RoomAliasesEvent, "m.room.aliases", - QStringList, aliases) - DEFINE_EVENTTYPE_ALIAS(RoomAliases, RoomAliasesEvent) DEFINE_SIMPLE_STATE_EVENT(RoomCanonicalAliasEvent, "m.room.canonical_alias", QString, alias) DEFINE_EVENTTYPE_ALIAS(RoomCanonicalAlias, RoomCanonicalAliasEvent) @@ -89,4 +84,21 @@ namespace QMatrixClient DEFINE_SIMPLE_STATE_EVENT(EncryptionEvent, "m.room.encryption", QString, algorithm) DEFINE_EVENTTYPE_ALIAS(RoomEncryption, EncryptionEvent) + + class RoomAliasesEvent + : public StateEvent> + { + public: + DEFINE_EVENT_TYPEID("m.room.aliases", RoomAliasesEvent) + explicit RoomAliasesEvent(const QJsonObject& obj) + : StateEvent(typeId(), obj, QStringLiteral("aliases")) + { } + RoomAliasesEvent(const QString& server, const QStringList& aliases) + : StateEvent(typeId(), matrixTypeId(), server, + QStringLiteral("aliases"), aliases) + { } + QString server() const { return stateKey(); } + QStringList aliases() const { return content().value; } + }; + REGISTER_EVENT_TYPE(RoomAliasesEvent) } // namespace QMatrixClient diff --git a/lib/events/stateevent.cpp b/lib/events/stateevent.cpp index 476e0fd6..6a6e7782 100644 --- a/lib/events/stateevent.cpp +++ b/lib/events/stateevent.cpp @@ -36,6 +36,12 @@ using namespace QMatrixClient; return makeEvent(unknownEventTypeId(), json); }); +StateEventBase::StateEventBase(Event::Type type, event_mtype_t matrixType, + const QString &stateKey, + const QJsonObject &contentJson) + : RoomEvent(type, basicStateEventJson(matrixType, contentJson, stateKey)) +{ } + bool StateEventBase::repeatsState() const { const auto prevContentJson = unsignedJson().value(PrevContentKeyL); diff --git a/lib/events/stateevent.h b/lib/events/stateevent.h index 5dadac7f..692f2685 100644 --- a/lib/events/stateevent.h +++ b/lib/events/stateevent.h @@ -37,7 +37,12 @@ namespace QMatrixClient { public: using factory_t = EventFactory; - using RoomEvent::RoomEvent; + StateEventBase(Type type, const QJsonObject& json) + : RoomEvent(type, json) + { } + StateEventBase(Type type, event_mtype_t matrixType, + const QString& stateKey = {}, + const QJsonObject& contentJson = {}); ~StateEventBase() override = default; bool isStateEvent() const override { return true; } @@ -94,8 +99,9 @@ namespace QMatrixClient { } template explicit StateEvent(Type type, event_mtype_t matrixType, + const QString& stateKey = {}, ContentParamTs&&... contentParams) - : StateEventBase(type, matrixType) + : StateEventBase(type, matrixType, stateKey) , _content(std::forward(contentParams)...) { editJson().insert(ContentKey, _content.toJson()); diff --git a/lib/room.cpp b/lib/room.cpp index 8c9f8760..44cd0d06 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -93,6 +93,8 @@ class Room::Private /// The state of the room at timeline position before-0 /// \sa timelineBase std::unordered_map baseState; + /// State event stubs - events without content, just type and state key + static decltype(baseState) stubbedState; /// The state of the room at timeline position after-maxTimelineIndex() /// \sa Room::syncEdge QHash currentState; @@ -193,9 +195,22 @@ class Room::Private template const EventT* getCurrentState(const QString& stateKey = {}) const { - static const EventT empty; - const auto* evt = - currentState.value({EventT::matrixTypeId(), stateKey}, &empty); + const StateEventKey evtKey { EventT::matrixTypeId(), stateKey }; + const auto* evt = currentState.value(evtKey, nullptr); + if (!evt) { + if (stubbedState.find(evtKey) == stubbedState.end()) { + // In the absence of a real event, make a stub as-if an event + // with empty content has been received. Event classes should be + // prepared for empty/invalid/malicious content anyway. + stubbedState.emplace(evtKey, + loadStateEvent(EventT::matrixTypeId(), + {}, stateKey)); + qCDebug(MAIN) << "A new stub event created for key {" + << evtKey.first << evtKey.second << "}"; + } + evt = stubbedState[evtKey].get(); + Q_ASSERT(evt); + } Q_ASSERT(evt->type() == EventT::typeId() && evt->matrixType() == EventT::matrixTypeId()); return static_cast(evt); @@ -272,28 +287,26 @@ class Room::Private QString doSendEvent(const RoomEvent* pEvent); void onEventSendingFailure(const QString& txnId, BaseJob* call = nullptr); - template - SetRoomStateWithKeyJob* requestSetState(const QString& stateKey, - const EvT& event) + SetRoomStateWithKeyJob* requestSetState(const StateEventBase& event) { if (q->successorId().isEmpty()) { // TODO: Queue up state events sending (see #133). return connection->callApi( - id, EvT::matrixTypeId(), stateKey, event.contentJson()); + id, event.matrixType(), + event.stateKey(), event.contentJson()); } qCWarning(MAIN) << q << "has been upgraded, state won't be set"; return nullptr; } - template - auto requestSetState(const EvT& event) + template + auto requestSetState(ArgTs&&... args) { - return connection->callApi( - id, EvT::matrixTypeId(), event.contentJson()); + return requestSetState(EvT(std::forward(args)...)); } - /** + /** * @brief Apply redaction to the timeline * * Tries to find an event in the timeline and redact it; deletes the @@ -317,6 +330,8 @@ class Room::Private } }; +decltype(Room::Private::baseState) Room::Private::stubbedState { }; + Room::Room(Connection* connection, QString id, JoinState initialJoinState) : QObject(connection), d(new Private(connection, id, initialJoinState)) { @@ -1625,28 +1640,32 @@ QString Room::postEvent(RoomEvent* event) QString Room::postJson(const QString& matrixType, const QJsonObject& eventContent) { - return d->sendEvent(loadEvent(basicEventJson(matrixType, eventContent))); + return d->sendEvent(loadEvent(matrixType, eventContent)); +} + +SetRoomStateWithKeyJob* Room::setState(const StateEventBase& evt) const { + return d->requestSetState(evt); } void Room::setName(const QString& newName) { - d->requestSetState(RoomNameEvent(newName)); + d->requestSetState(newName); } void Room::setCanonicalAlias(const QString& newAlias) { - d->requestSetState(RoomCanonicalAliasEvent(newAlias)); + d->requestSetState(newAlias); } void Room::setLocalAliases(const QStringList& aliases) { - d->requestSetState(connection()->homeserver().authority(), - RoomAliasesEvent(aliases)); + d->requestSetState( + connection()->homeserver().authority(), aliases); } void Room::setTopic(const QString& newTopic) { - d->requestSetState(RoomTopicEvent(newTopic)); + d->requestSetState(newTopic); } bool isEchoEvent(const RoomEventPtr& le, const PendingEventItem& re) @@ -1756,9 +1775,10 @@ LeaveRoomJob* Room::leaveRoom() return connection()->leaveRoom(this); } -SetRoomStateWithKeyJob*Room::setMemberState(const QString& memberId, const RoomMemberEvent& event) const +SetRoomStateWithKeyJob* Room::setMemberState( + const QString& memberId, const RoomMemberEvent& event) const { - return d->requestSetState(memberId, event); + return d->requestSetState(memberId, event.content()); } void Room::kickMember(const QString& memberId, const QString& reason) diff --git a/lib/room.h b/lib/room.h index 7c85e4ed..3abf262d 100644 --- a/lib/room.h +++ b/lib/room.h @@ -440,6 +440,9 @@ namespace QMatrixClient const QJsonObject& eventContent); QString retryMessage(const QString& txnId); void discardMessage(const QString& txnId); + + /// Send a request to update the room state with the given event + SetRoomStateWithKeyJob* setState(const StateEventBase& evt) const; void setName(const QString& newName); void setCanonicalAlias(const QString& newAlias); /// Set room aliases on the user's current server @@ -453,6 +456,7 @@ namespace QMatrixClient void inviteToRoom(const QString& memberId); LeaveRoomJob* leaveRoom(); + /// \deprecated - use setState() instead") SetRoomStateWithKeyJob* setMemberState( const QString& memberId, const RoomMemberEvent& event) const; void kickMember(const QString& memberId, const QString& reason = {}); diff --git a/lib/user.cpp b/lib/user.cpp index 7b695618..8bdcbe97 100644 --- a/lib/user.cpp +++ b/lib/user.cpp @@ -292,7 +292,7 @@ void User::rename(const QString& newName, const Room* r) const auto actualNewName = sanitized(newName); MemberEventContent evtC; evtC.displayName = actualNewName; - connect(r->setMemberState(id(), RoomMemberEvent(move(evtC))), + connect(r->setState(RoomMemberEvent(id(), move(evtC))), &BaseJob::success, this, [=] { updateName(actualNewName, r); }); } -- cgit v1.2.3