Disallow direct events construction from JSON

Direct construction (using makeEvent() or explicitly constructing
an event) from JSON may create an event that has a type conflicting
with that stored in JSON. There's no such problem with loadEvent(),
even though it's considerably slower. Driven by the fact that almost
nowhere in the code direct construction is used on checked JSON
(one test is the only valid case), this commit moves all JSON-loading
constructors to the protected section, thereby disabling usage of
makeEvent() in JSON-loading capacity, and switches such cases across
the library to loadEvent().

7 files changed, 39 insertions, 24 deletions

diff --git a/autotests/callcandidateseventtest.cpp b/autotests/callcandidateseventtest.cpp
index b37dd109..257e0ef2 100644
--- a/autotests/callcandidateseventtest.cpp
+++ b/autotests/callcandidateseventtest.cpp
@@ -40,13 +40,16 @@ void TestCallCandidatesEvent::fromJson()
 
     auto object = document.object();
 
-    Quotient::CallCandidatesEvent callCandidatesEvent(object);
+    using namespace Quotient;
+    const auto& callCandidatesEvent = loadEvent<CallCandidatesEvent>(object);
+    QVERIFY(callCandidatesEvent);
+    QVERIFY(callCandidatesEvent->is<CallCandidatesEvent>());
 
-    QCOMPARE(callCandidatesEvent.version(), 0);
-    QCOMPARE(callCandidatesEvent.callId(), QStringLiteral("12345"));
-    QCOMPARE(callCandidatesEvent.candidates().count(), 1);
+    QCOMPARE(callCandidatesEvent->version(), 0);
+    QCOMPARE(callCandidatesEvent->callId(), QStringLiteral("12345"));
+    QCOMPARE(callCandidatesEvent->candidates().count(), 1);
 
-    const QJsonObject &candidate = callCandidatesEvent.candidates().at(0).toObject();
+    const auto& candidate = callCandidatesEvent->candidates().at(0).toObject();
     QCOMPARE(candidate.value("sdpMid").toString(), QStringLiteral("audio"));
     QCOMPARE(candidate.value("sdpMLineIndex").toInt(), 0);
     QCOMPARE(candidate.value("candidate").toString(),
diff --git a/lib/connection.cpp b/lib/connection.cpp
index 471dc20d..a33ace51 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -2242,10 +2242,12 @@ void Connection::saveOlmAccount()
 #ifdef Quotient_E2EE_ENABLED
 QJsonObject Connection::decryptNotification(const QJsonObject &notification)
 {
-    auto r = room(notification["room_id"].toString());
-    auto event = makeEvent<EncryptedEvent>(notification["event"].toObject());
-    const auto decrypted = r->decryptMessage(*event);
-    return decrypted ? decrypted->fullJson() : QJsonObject();
+    if (auto r = room(notification["room_id"].toString()))
+        if (auto event =
+                loadEvent<EncryptedEvent>(notification["event"].toObject()))
+            if (const auto decrypted = r->decryptMessage(*event))
+                return decrypted->fullJson();
+    return QJsonObject();
 }
 
 Database* Connection::database() const
diff --git a/lib/events/callevents.h b/lib/events/callevents.h
index 6d9feae4..752e331d 100644
--- a/lib/events/callevents.h
+++ b/lib/events/callevents.h
@@ -15,12 +15,12 @@ public:
         return mType.startsWith("m.call.");
     }
 
-    explicit CallEvent(const QJsonObject& json);
-
     QUO_CONTENT_GETTER(QString, callId)
     QUO_CONTENT_GETTER(int, version)
 
 protected:
+    explicit CallEvent(const QJsonObject& json);
+
     static QJsonObject basicJson(const QString& matrixType,
                                  const QString& callId, int version,
                                  QJsonObject contentJson = {});
diff --git a/lib/events/event.h b/lib/events/event.h
index 6a7acf28..9d7c61a9 100644
--- a/lib/events/event.h
+++ b/lib/events/event.h
@@ -203,6 +203,9 @@ private:
 // === Event creation facilities ===
 
 //! \brief Create an event of arbitrary type from its arguments
+//!
+//! This should not be used to load events from JSON - use loadEvent() for that.
+//! \sa loadEvent
 template <typename EventT, typename... ArgTs>
 inline event_ptr_tt<EventT> makeEvent(ArgTs&&... args)
 {
@@ -266,8 +269,6 @@ public:
         return BaseMetaType;
     }
 
-    explicit Event(const QJsonObject& json);
-
     Q_DISABLE_COPY(Event)
     Event(Event&&) noexcept = default;
     Event& operator=(Event&&) = delete;
@@ -364,6 +365,10 @@ public:
     [[deprecated("Use is<CallEvent>() instead")]] bool isCallEvent() const;
 
 protected:
+    friend class EventMetaType<Event>; // To access the below constructor
+
+    explicit Event(const QJsonObject& json);
+
     QJsonObject& editJson() { return _json; }
     virtual void dumpTo(QDebug dbg) const;
 
@@ -427,6 +432,7 @@ public:
 //! pointing to that BaseMetaType.
 //! \sa EventMetaType, EventMetaType::SuppressLoadDerived
 #define QUO_BASE_EVENT(CppType_, ...)                      \
+    friend class EventMetaType<CppType_>;                  \
     static inline EventMetaType<CppType_> BaseMetaType{    \
         #CppType_ __VA_OPT__(,) __VA_ARGS__ };             \
     const AbstractEventMetaType& metaType() const override \
@@ -452,6 +458,7 @@ public:
 //! \sa EventMetaType
 #define QUO_EVENT(CppType_, MatrixType_, ...)                           \
     static inline const auto& TypeId = MatrixType_##_ls;                \
+    friend class EventMetaType<CppType_>;                               \
     static inline const EventMetaType<CppType_> MetaType{               \
         #CppType_, TypeId, BaseMetaType __VA_OPT__(,) __VA_ARGS__       \
     };                                                                  \
diff --git a/lib/events/roomevent.cpp b/lib/events/roomevent.cpp
index 8928c81c..e98cb591 100644
--- a/lib/events/roomevent.cpp
+++ b/lib/events/roomevent.cpp
@@ -12,7 +12,7 @@ RoomEvent::RoomEvent(const QJsonObject& json) : Event(json)
 {
     if (const auto redaction = unsignedPart<QJsonObject>(RedactedCauseKeyL);
         !redaction.isEmpty())
-        _redactedBecause = makeEvent<RedactionEvent>(redaction);
+        _redactedBecause = loadEvent<RedactionEvent>(redaction);
 }
 
 RoomEvent::~RoomEvent() = default; // Let the smart pointer do its job
diff --git a/lib/events/roomevent.h b/lib/events/roomevent.h
index 47b0b59d..203434f6 100644
--- a/lib/events/roomevent.h
+++ b/lib/events/roomevent.h
@@ -16,10 +16,7 @@ class QUOTIENT_API RoomEvent : public Event {
 public:
     QUO_BASE_EVENT(RoomEvent, {}, Event::BaseMetaType)
 
-    // RedactionEvent is an incomplete type here so we cannot inline
-    // constructors using it and also destructors (with 'using', in particular).
-    explicit RoomEvent(const QJsonObject& json);
-    ~RoomEvent() override;
+    ~RoomEvent() override; // Don't inline this - see the private section
 
     QString id() const;
     QDateTime originTimestamp() const;
@@ -66,9 +63,12 @@ public:
 #endif
 
 protected:
+    explicit RoomEvent(const QJsonObject& json);
     void dumpTo(QDebug dbg) const override;
 
 private:
+    // RedactionEvent is an incomplete type here so we cannot inline
+    // constructors using it and also destructors (with 'using', in particular).
     event_ptr_tt<RedactionEvent> _redactedBecause;
 
 #ifdef Quotient_E2EE_ENABLED
diff --git a/lib/events/stateevent.h b/lib/events/stateevent.h
index 911972f2..ffbce76e 100644
--- a/lib/events/stateevent.h
+++ b/lib/events/stateevent.h
@@ -24,7 +24,6 @@ public:
     //! constructors and calls in, e.g., RoomStateView don't include it.
     static constexpr auto needsStateKey = false;
 
-    explicit StateEventBase(const QJsonObject& json);
     explicit StateEventBase(Type type, const QString& stateKey = {},
                             const QJsonObject& contentJson = {});
 
@@ -39,9 +38,11 @@ public:
     }
 
     QString replacedState() const;
-    void dumpTo(QDebug dbg) const override;
-
     virtual bool repeatsState() const;
+
+protected:
+    explicit StateEventBase(const QJsonObject& json);
+    void dumpTo(QDebug dbg) const override;
 };
 using StateEventPtr = event_ptr_tt<StateEventBase>;
 using StateEvents = EventsArray<StateEventBase>;
@@ -129,13 +130,15 @@ private:
     using base_type = EventTemplate<EventT, StateEventBase, ContentT>;
 
 public:
-    explicit KeylessStateEventBase(const QJsonObject& fullJson)
-        : base_type(fullJson)
-    {}
     template <typename... ContentParamTs>
     explicit KeylessStateEventBase(ContentParamTs&&... contentParams)
         : base_type(QString(), std::forward<ContentParamTs>(contentParams)...)
     {}
+
+protected:
+    explicit KeylessStateEventBase(const QJsonObject& fullJson)
+        : base_type(fullJson)
+    {}
 };
 
 template <typename EvT>