Dima/matrix-js-sdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix wrong handling of encrypted rooms when loading them from sync accumulator (#3640)

Browse Source 
* Revert "Ensure we don't overinflate the total notification count (#3634)"

This reverts commit fd0c4a7f56.

* Fix wrong handling of encrypted rooms when loading them from sync accumulator

* Tidy up code, removing sections which didn't make any difference

* Add test
This commit is contained in:
Michael Telatynski
2023-08-02 10:53:34 +01:00
committed by GitHub
parent 2e9b34e0c3
commit 2172f28888
3 changed files with 95 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spec/integ/matrix-client-syncing.spec.ts
+63
View File
@@ -38,6 +38,7 @@ import {
Room,
IndexedDBStore,
RelationType,
EventType,
} from "../../src";
import { ReceiptType } from "../../src/@types/read_receipts";
import { UNREAD_THREAD_NOTIFICATIONS } from "../../src/@types/sync";
@@ -1590,6 +1591,68 @@ describe("MatrixClient syncing", () => {
});
});
});
it("should apply encrypted notification logic for events within the same sync blob", async () => {
const roomId = "!room123:server";
const syncData = {
rooms: {
join: {
[roomId]: {
ephemeral: {
events: [],
},
timeline: {
events: [
utils.mkEvent({
room: roomId,
event: true,
skey: "",
type: EventType.RoomEncryption,
content: {},
}),
utils.mkMessage({
room: roomId,
user: otherUserId,
msg: "hello",
}),
],
},
state: {
events: [
utils.mkMembership({
room: roomId,
mship: "join",
user: otherUserId,
}),
utils.mkMembership({
room: roomId,
mship: "join",
user: selfUserId,
}),
utils.mkEvent({
type: "m.room.create",
room: roomId,
user: selfUserId,
content: {
creator: selfUserId,
},
}),
],
},
},
},
},
} as unknown as ISyncResponse;
httpBackend!.when("GET", "/sync").respond(200, syncData);
client!.startClient();
await Promise.all([httpBackend!.flushAllExpected(), awaitSyncEvent()]);
const room = client!.getRoom(roomId)!;
expect(room).toBeInstanceOf(Room);
expect(room.getRoomUnreadNotificationCount(NotificationCountType.Total)).toBe(0);
});
});
describe("of a room", () => {
src/client.ts
+13 -25
View File
@@ -9862,19 +9862,8 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri
const room = cli.getRoom(event.getRoomId());
if (!room || !ourUserId || !eventId) return;
const oldActions = event.getPushActions();
const actions = cli.getPushActionsForEvent(event, true);
const isThreadEvent = !!event.threadRootId && !event.isThreadRoot;
const currentHighlightCount = room.getUnreadCountForEventContext(NotificationCountType.Highlight, event);
// Ensure the unread counts are kept up to date if the event is encrypted
// We also want to make sure that the notification count goes up if we already
// have encrypted events to avoid other code from resetting 'highlight' to zero.
const oldHighlight = !!oldActions?.tweaks?.highlight;
const newHighlight = !!actions?.tweaks?.highlight;
let hasReadEvent;
if (isThreadEvent) {
const thread = room.getThread(event.threadRootId);
@@ -9896,13 +9885,17 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri
return;
}
if (oldHighlight !== newHighlight || currentHighlightCount > 0) {
const actions = cli.getPushActionsForEvent(event, true);
// Ensure the unread counts are kept up to date if the event is encrypted
// We also want to make sure that the notification count goes up if we already
// have encrypted events to avoid other code from resetting 'highlight' to zero.
const newHighlight = !!actions?.tweaks?.highlight;
if (newHighlight) {
// TODO: Handle mentions received while the client is offline
// See also https://github.com/vector-im/element-web/issues/9069
let newCount = currentHighlightCount;
if (newHighlight && !oldHighlight) newCount++;
if (!newHighlight && oldHighlight) newCount--;
const newCount = room.getUnreadCountForEventContext(NotificationCountType.Highlight, event) + 1;
if (isThreadEvent) {
room.setThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Highlight, newCount);
} else {
@@ -9910,23 +9903,18 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri
}
}
// Total count is used to typically increment a room notification counter, but not loudly highlight it.
const currentTotalCount = room.getUnreadCountForEventContext(NotificationCountType.Total, event);
// `notify` is used in practice for incrementing the total count
const newNotify = !!actions?.notify;
// The room total count is NEVER incremented by the server for encrypted rooms. We basically ignore
// the server here as it's always going to tell us to increment for encrypted events.
if (newNotify) {
// Total count is used to typically increment a room notification counter, but not loudly highlight it.
const newCount = room.getUnreadCountForEventContext(NotificationCountType.Total, event) + 1;
if (isThreadEvent) {
room.setThreadUnreadNotificationCount(
event.threadRootId,
NotificationCountType.Total,
currentTotalCount + 1,
);
room.setThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Total, newCount);
} else {
room.setUnreadNotificationCount(NotificationCountType.Total, currentTotalCount + 1);
room.setUnreadNotificationCount(NotificationCountType.Total, newCount);
}
}
}
src/sync.ts
+19 -3
View File
@@ -28,8 +28,7 @@ import { Optional } from "matrix-events-sdk";
import type { SyncCryptoCallbacks } from "./common-crypto/CryptoBackend";
import { User, UserEvent } from "./models/user";
import { NotificationCountType, Room, RoomEvent } from "./models/room";
import { promiseMapSeries, defer, deepCopy } from "./utils";
import { IDeferred, noUnsafeEventProps, unsafeProp } from "./utils";
import { deepCopy, defer, IDeferred, noUnsafeEventProps, promiseMapSeries, unsafeProp } from "./utils";
import { Filter } from "./filter";
import { EventTimeline } from "./models/event-timeline";
import { logger } from "./logger";
@@ -1316,7 +1315,7 @@ export class SyncApi {
const ephemeralEvents = this.mapSyncEventsFormat(joinObj.ephemeral);
const accountDataEvents = this.mapSyncEventsFormat(joinObj.account_data);
const encrypted = client.isRoomEncrypted(room.roomId);
const encrypted = this.isRoomEncrypted(room, stateEvents, events);
// We store the server-provided value first so it's correct when any of the events fire.
if (joinObj.unread_notifications) {
/**
@@ -1324,6 +1323,9 @@ export class SyncApi {
* bother setting it here. We trust our calculations better than the
* server's for this case, and therefore will assume that our non-zero
* count is accurate.
* XXX: this is known faulty as the push rule for `.m.room.encrypted` may be disabled so server
* may issue notification counts of 0 which we wrongly trust.
* https://github.com/matrix-org/matrix-spec-proposals/pull/2654 would fix this
*
* @see import("./client").fixNotificationCountOnDecryption
*/
@@ -1721,6 +1723,20 @@ export class SyncApi {
});
}
private findEncryptionEvent(events?: MatrixEvent[]): MatrixEvent | undefined {
return events?.find((e) => e.getType() === EventType.RoomEncryption && e.getStateKey() === "");
}
// When processing the sync response we cannot rely on MatrixClient::isRoomEncrypted before we actually
// inject the events into the room object, so we have to inspect the events themselves.
private isRoomEncrypted(room: Room, stateEventList: MatrixEvent[], timelineEventList?: MatrixEvent[]): boolean {
return (
this.client.isRoomEncrypted(room.roomId) ||
!!this.findEncryptionEvent(stateEventList) ||
!!this.findEncryptionEvent(timelineEventList)
);
}
/**
* Injects events into a room's model.
* @param stateEventList - A list of state events. This is the state