Don't access event after it was deleted

- NeoChat stores pointer to event
- Event is replaced
- libQuotient deletes the event and notifies us that the event changes
- We're accessing the event to check its id
- Boom

Make this not boom by accessing the ID that we're additionally storing anyway.
This commit is contained in:
Tobias Fella
2024-07-02 19:08:59 +02:00
parent c48b9874bf
commit d91ed535ad
2 changed files with 16 additions and 14 deletions

View File

@@ -34,6 +34,7 @@ MessageContentModel::MessageContentModel(NeoChatRoom *room, const Quotient::Room
: QAbstractListModel(nullptr) : QAbstractListModel(nullptr)
, m_room(room) , m_room(room)
, m_eventId(event != nullptr ? event->id() : QString()) , m_eventId(event != nullptr ? event->id() : QString())
, m_eventSenderId(event != nullptr ? event->senderId() : QString())
, m_event(event) , m_event(event)
, m_isPending(isPending) , m_isPending(isPending)
, m_isReply(isReply) , m_isReply(isReply)
@@ -77,7 +78,7 @@ void MessageContentModel::initializeModel()
connect(m_room, &NeoChatRoom::pendingEventAboutToMerge, this, [this](Quotient::RoomEvent *serverEvent) { connect(m_room, &NeoChatRoom::pendingEventAboutToMerge, this, [this](Quotient::RoomEvent *serverEvent) {
if (m_room != nullptr && m_event != nullptr) { if (m_room != nullptr && m_event != nullptr) {
if (m_event->id() == serverEvent->id()) { if (m_eventId == serverEvent->id()) {
beginResetModel(); beginResetModel();
m_isPending = false; m_isPending = false;
m_event = serverEvent; m_event = serverEvent;
@@ -88,7 +89,7 @@ void MessageContentModel::initializeModel()
}); });
connect(m_room, &NeoChatRoom::replacedEvent, this, [this](const Quotient::RoomEvent *newEvent) { connect(m_room, &NeoChatRoom::replacedEvent, this, [this](const Quotient::RoomEvent *newEvent) {
if (m_room != nullptr && m_event != nullptr) { if (m_room != nullptr && m_event != nullptr) {
if (m_event->id() == newEvent->id()) { if (m_eventId == newEvent->id()) {
beginResetModel(); beginResetModel();
m_event = newEvent; m_event = newEvent;
Q_EMIT eventUpdated(); Q_EMIT eventUpdated();
@@ -97,17 +98,17 @@ void MessageContentModel::initializeModel()
} }
}); });
connect(m_room, &NeoChatRoom::newFileTransfer, this, [this](const QString &eventId) { connect(m_room, &NeoChatRoom::newFileTransfer, this, [this](const QString &eventId) {
if (m_event != nullptr && eventId == m_event->id()) { if (m_event != nullptr && eventId == m_eventId) {
Q_EMIT dataChanged(index(0), index(rowCount() - 1), {FileTransferInfoRole}); Q_EMIT dataChanged(index(0), index(rowCount() - 1), {FileTransferInfoRole});
} }
}); });
connect(m_room, &NeoChatRoom::fileTransferProgress, this, [this](const QString &eventId) { connect(m_room, &NeoChatRoom::fileTransferProgress, this, [this](const QString &eventId) {
if (m_event != nullptr && eventId == m_event->id()) { if (m_event != nullptr && eventId == m_eventId) {
Q_EMIT dataChanged(index(0), index(rowCount() - 1), {FileTransferInfoRole}); Q_EMIT dataChanged(index(0), index(rowCount() - 1), {FileTransferInfoRole});
} }
}); });
connect(m_room, &NeoChatRoom::fileTransferCompleted, this, [this](const QString &eventId) { connect(m_room, &NeoChatRoom::fileTransferCompleted, this, [this](const QString &eventId) {
if (m_event != nullptr && eventId == m_event->id()) { if (m_event != nullptr && eventId == m_eventId) {
updateComponents(); updateComponents();
Q_EMIT dataChanged(index(0), index(rowCount() - 1), {FileTransferInfoRole}); Q_EMIT dataChanged(index(0), index(rowCount() - 1), {FileTransferInfoRole});
@@ -122,22 +123,22 @@ void MessageContentModel::initializeModel()
if (mxcUrl.isEmpty()) { if (mxcUrl.isEmpty()) {
return; return;
} }
auto localPath = m_room->fileTransferInfo(m_event->id()).localPath.toLocalFile(); auto localPath = m_room->fileTransferInfo(m_eventId).localPath.toLocalFile();
auto config = KSharedConfig::openStateConfig(QStringLiteral("neochatdownloads"))->group(QStringLiteral("downloads")); auto config = KSharedConfig::openStateConfig(QStringLiteral("neochatdownloads"))->group(QStringLiteral("downloads"));
config.writePathEntry(mxcUrl.mid(6), localPath); config.writePathEntry(mxcUrl.mid(6), localPath);
} }
}); });
connect(m_room, &NeoChatRoom::fileTransferFailed, this, [this](const QString &eventId) { connect(m_room, &NeoChatRoom::fileTransferFailed, this, [this](const QString &eventId) {
if (m_event != nullptr && eventId == m_event->id()) { if (m_event != nullptr && eventId == m_eventId) {
updateComponents(); updateComponents();
Q_EMIT dataChanged(index(0), index(rowCount() - 1), {FileTransferInfoRole}); Q_EMIT dataChanged(index(0), index(rowCount() - 1), {FileTransferInfoRole});
} }
}); });
connect(m_room->editCache(), &ChatBarCache::relationIdChanged, this, [this](const QString &oldEventId, const QString &newEventId) { connect(m_room->editCache(), &ChatBarCache::relationIdChanged, this, [this](const QString &oldEventId, const QString &newEventId) {
if (m_event != nullptr && (oldEventId == m_event->id() || newEventId == m_event->id())) { if (m_event != nullptr && (oldEventId == m_eventId || newEventId == m_eventId)) {
// HACK: Because DelegateChooser can't switch the delegate on dataChanged it has to think there is a new delegate. // HACK: Because DelegateChooser can't switch the delegate on dataChanged it has to think there is a new delegate.
beginResetModel(); beginResetModel();
updateComponents(newEventId == m_event->id()); updateComponents(newEventId == m_eventId);
endResetModel(); endResetModel();
} }
}); });
@@ -149,14 +150,14 @@ void MessageContentModel::initializeModel()
}); });
connect(m_room, &Room::memberNameUpdated, this, [this](RoomMember member) { connect(m_room, &Room::memberNameUpdated, this, [this](RoomMember member) {
if (m_room != nullptr && m_event != nullptr) { if (m_room != nullptr && m_event != nullptr) {
if (m_event->senderId() == member.id()) { if (m_eventSenderId.isEmpty() || m_eventSenderId == member.id()) {
Q_EMIT dataChanged(index(0, 0), index(rowCount() - 1, 0), {AuthorRole}); Q_EMIT dataChanged(index(0, 0), index(rowCount() - 1, 0), {AuthorRole});
} }
} }
}); });
connect(m_room, &Room::memberAvatarUpdated, this, [this](RoomMember member) { connect(m_room, &Room::memberAvatarUpdated, this, [this](RoomMember member) {
if (m_room != nullptr && m_event != nullptr) { if (m_room != nullptr && m_event != nullptr) {
if (m_event->senderId() == member.id()) { if (m_eventSenderId.isEmpty() || m_eventSenderId == member.id()) {
Q_EMIT dataChanged(index(0, 0), index(rowCount() - 1, 0), {AuthorRole}); Q_EMIT dataChanged(index(0, 0), index(rowCount() - 1, 0), {AuthorRole});
} }
} }
@@ -264,7 +265,7 @@ QVariant MessageContentModel::data(const QModelIndex &index, int role) const
return eventHandler.getLocationAssetType(); return eventHandler.getLocationAssetType();
} }
if (role == PollHandlerRole) { if (role == PollHandlerRole) {
return QVariant::fromValue<PollHandler *>(m_room->poll(m_event->id())); return QVariant::fromValue<PollHandler *>(m_room->poll(m_eventId));
} }
if (role == IsReplyRole) { if (role == IsReplyRole) {
return eventHandler.hasReply(); return eventHandler.hasReply();
@@ -560,13 +561,13 @@ FileTransferInfo MessageContentModel::fileInfo() const
} }
auto config = KSharedConfig::openStateConfig(QStringLiteral("neochatdownloads"))->group(QStringLiteral("downloads")); auto config = KSharedConfig::openStateConfig(QStringLiteral("neochatdownloads"))->group(QStringLiteral("downloads"));
if (!config.hasKey(mxcUrl.mid(6))) { if (!config.hasKey(mxcUrl.mid(6))) {
return m_room->fileTransferInfo(m_event->id()); return m_room->fileTransferInfo(m_eventId);
} }
const auto path = config.readPathEntry(mxcUrl.mid(6), QString()); const auto path = config.readPathEntry(mxcUrl.mid(6), QString());
QFileInfo info(path); QFileInfo info(path);
if (!info.isFile()) { if (!info.isFile()) {
config.deleteEntry(mxcUrl); config.deleteEntry(mxcUrl);
return m_room->fileTransferInfo(m_event->id()); return m_room->fileTransferInfo(m_eventId);
} }
// TODO: we could check the hash here // TODO: we could check the hash here
return FileTransferInfo{ return FileTransferInfo{

View File

@@ -114,6 +114,7 @@ Q_SIGNALS:
private: private:
QPointer<NeoChatRoom> m_room; QPointer<NeoChatRoom> m_room;
QString m_eventId; QString m_eventId;
QString m_eventSenderId;
const Quotient::RoomEvent *m_event = nullptr; const Quotient::RoomEvent *m_event = nullptr;
bool m_isPending; bool m_isPending;