Fix account & room settings race condition (#7953)

pull/21833/head
Michael Telatynski 2022-03-03 22:09:06 +00:00 committed by GitHub
parent b8f37a46f0
commit afbe3d16b4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 169 additions and 122 deletions

View File

@ -17,8 +17,8 @@ limitations under the License.
import { ClientEvent, MatrixClient } from "matrix-js-sdk/src/client"; import { ClientEvent, MatrixClient } from "matrix-js-sdk/src/client";
import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { MatrixEvent } from "matrix-js-sdk/src/models/event";
import { defer } from "matrix-js-sdk/src/utils";
import { MatrixClientPeg } from '../../MatrixClientPeg';
import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler"; import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler";
import { objectClone, objectKeyChanges } from "../../utils/objects"; import { objectClone, objectKeyChanges } from "../../utils/objects";
import { SettingLevel } from "../SettingLevel"; import { SettingLevel } from "../SettingLevel";
@ -30,6 +30,7 @@ const BREADCRUMBS_EVENT_TYPES = [BREADCRUMBS_LEGACY_EVENT_TYPE, BREADCRUMBS_EVEN
const RECENT_EMOJI_EVENT_TYPE = "io.element.recent_emoji"; const RECENT_EMOJI_EVENT_TYPE = "io.element.recent_emoji";
const INTEG_PROVISIONING_EVENT_TYPE = "im.vector.setting.integration_provisioning"; const INTEG_PROVISIONING_EVENT_TYPE = "im.vector.setting.integration_provisioning";
const ANALYTICS_EVENT_TYPE = "im.vector.analytics"; const ANALYTICS_EVENT_TYPE = "im.vector.analytics";
const DEFAULT_SETTINGS_EVENT_TYPE = "im.vector.web.settings";
/** /**
* Gets and sets settings at the "account" level for the current user. * Gets and sets settings at the "account" level for the current user.
@ -45,10 +46,7 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa
} }
public initMatrixClient(oldClient: MatrixClient, newClient: MatrixClient) { public initMatrixClient(oldClient: MatrixClient, newClient: MatrixClient) {
if (oldClient) { oldClient?.removeListener(ClientEvent.AccountData, this.onAccountData);
oldClient.removeListener(ClientEvent.AccountData, this.onAccountData);
}
newClient.on(ClientEvent.AccountData, this.onAccountData); newClient.on(ClientEvent.AccountData, this.onAccountData);
} }
@ -62,9 +60,9 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa
} }
this.watchers.notifyUpdate("urlPreviewsEnabled", null, SettingLevel.ACCOUNT, val); this.watchers.notifyUpdate("urlPreviewsEnabled", null, SettingLevel.ACCOUNT, val);
} else if (event.getType() === "im.vector.web.settings" || event.getType() === ANALYTICS_EVENT_TYPE) { } else if (event.getType() === DEFAULT_SETTINGS_EVENT_TYPE || event.getType() === ANALYTICS_EVENT_TYPE) {
// Figure out what changed and fire those updates // Figure out what changed and fire those updates
const prevContent = prevEvent ? prevEvent.getContent() : {}; const prevContent = prevEvent?.getContent() ?? {};
const changedSettings = objectKeyChanges<Record<string, any>>(prevContent, event.getContent()); const changedSettings = objectKeyChanges<Record<string, any>>(prevContent, event.getContent());
for (const settingName of changedSettings) { for (const settingName of changedSettings) {
const val = event.getContent()[settingName]; const val = event.getContent()[settingName];
@ -136,56 +134,67 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa
return preferredValue; return preferredValue;
} }
public async setValue(settingName: string, roomId: string, newValue: any): Promise<void> { // helper function to set account data then await it being echoed back
// Special case URL previews private async setAccountData(
if (settingName === "urlPreviewsEnabled") { eventType: string,
const content = this.getSettings("org.matrix.preview_urls") || {}; field: string,
content['disable'] = !newValue; value: any,
await MatrixClientPeg.get().setAccountData("org.matrix.preview_urls", content); legacyEventType?: string,
return; ): Promise<void> {
let content = this.getSettings(eventType);
if (legacyEventType && !content?.[field]) {
content = this.getSettings(legacyEventType);
} }
// Special case for breadcrumbs if (!content) {
if (settingName === "breadcrumb_rooms") { content = {};
// We read the value first just to make sure we preserve whatever random keys might be present.
let content = this.getSettings(BREADCRUMBS_EVENT_TYPE);
if (!content || !content['recent_rooms']) {
content = this.getSettings(BREADCRUMBS_LEGACY_EVENT_TYPE);
}
if (!content) content = {}; // If we still don't have content, make some
content['recent_rooms'] = newValue;
await MatrixClientPeg.get().setAccountData(BREADCRUMBS_EVENT_TYPE, content);
return;
} }
// Special case recent emoji content[field] = value;
if (settingName === "recent_emoji") {
const content = this.getSettings(RECENT_EMOJI_EVENT_TYPE) || {};
content["recent_emoji"] = newValue;
await MatrixClientPeg.get().setAccountData(RECENT_EMOJI_EVENT_TYPE, content);
return;
}
// Special case integration manager provisioning await this.client.setAccountData(eventType, content);
if (settingName === "integrationProvisioning") {
const content = this.getSettings(INTEG_PROVISIONING_EVENT_TYPE) || {};
content['enabled'] = newValue;
await MatrixClientPeg.get().setAccountData(INTEG_PROVISIONING_EVENT_TYPE, content);
return;
}
// Special case analytics const deferred = defer<void>();
if (settingName === "pseudonymousAnalyticsOptIn") { const handler = (event: MatrixEvent) => {
const content = this.getSettings(ANALYTICS_EVENT_TYPE) || {}; if (event.getType() !== eventType || event.getContent()[field] !== value) return;
content[settingName] = newValue; this.client.off(ClientEvent.AccountData, handler);
await MatrixClientPeg.get().setAccountData(ANALYTICS_EVENT_TYPE, content); deferred.resolve();
return; };
} this.client.on(ClientEvent.AccountData, handler);
const content = this.getSettings() || {}; await deferred.promise;
content[settingName] = newValue; }
await MatrixClientPeg.get().setAccountData("im.vector.web.settings", content);
public setValue(settingName: string, roomId: string, newValue: any): Promise<void> {
switch (settingName) {
// Special case URL previews
case "urlPreviewsEnabled":
return this.setAccountData("org.matrix.preview_urls", "disable", !newValue);
// Special case for breadcrumbs
case "breadcrumb_rooms":
return this.setAccountData(
BREADCRUMBS_EVENT_TYPE,
"recent_rooms",
newValue,
BREADCRUMBS_LEGACY_EVENT_TYPE,
);
// Special case recent emoji
case "recent_emoji":
return this.setAccountData(RECENT_EMOJI_EVENT_TYPE, "recent_emoji", newValue);
// Special case integration manager provisioning
case "integrationProvisioning":
return this.setAccountData(INTEG_PROVISIONING_EVENT_TYPE, "enabled", newValue);
// Special case analytics
case "pseudonymousAnalyticsOptIn":
return this.setAccountData(ANALYTICS_EVENT_TYPE, "pseudonymousAnalyticsOptIn", newValue);
default:
return this.setAccountData(DEFAULT_SETTINGS_EVENT_TYPE, settingName, newValue);
}
} }
public canSetValue(settingName: string, roomId: string): boolean { public canSetValue(settingName: string, roomId: string): boolean {
@ -193,15 +202,13 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa
} }
public isSupported(): boolean { public isSupported(): boolean {
const cli = MatrixClientPeg.get(); return this.client && !this.client.isGuest();
return cli !== undefined && cli !== null && !cli.isGuest();
} }
private getSettings(eventType = "im.vector.web.settings"): any { // TODO: [TS] Types on return private getSettings(eventType = "im.vector.web.settings"): any { // TODO: [TS] Types on return
const cli = MatrixClientPeg.get(); if (!this.client) return null;
if (!cli) return null;
const event = cli.getAccountData(eventType); const event = this.client.getAccountData(eventType);
if (!event || !event.getContent()) return null; if (!event || !event.getContent()) return null;
return objectClone(event.getContent()); // clone to prevent mutation return objectClone(event.getContent()); // clone to prevent mutation
} }

View File

@ -40,31 +40,37 @@ export default class LocalEchoWrapper extends SettingsHandler {
} }
public getValue(settingName: string, roomId: string): any { public getValue(settingName: string, roomId: string): any {
const cacheRoomId = roomId ? roomId : "UNDEFINED"; // avoid weird keys const cacheRoomId = roomId ?? "UNDEFINED"; // avoid weird keys
const bySetting = this.cache[settingName]; const bySetting = this.cache[settingName];
if (bySetting && bySetting.hasOwnProperty(cacheRoomId)) { if (bySetting?.hasOwnProperty(cacheRoomId)) {
return bySetting[cacheRoomId]; return bySetting[cacheRoomId];
} }
return this.handler.getValue(settingName, roomId); return this.handler.getValue(settingName, roomId);
} }
public setValue(settingName: string, roomId: string, newValue: any): Promise<void> { public async setValue(settingName: string, roomId: string, newValue: any): Promise<void> {
if (!this.cache[settingName]) this.cache[settingName] = {}; if (!this.cache[settingName]) this.cache[settingName] = {};
const bySetting = this.cache[settingName]; const bySetting = this.cache[settingName];
const cacheRoomId = roomId ? roomId : "UNDEFINED"; // avoid weird keys const cacheRoomId = roomId ?? "UNDEFINED"; // avoid weird keys
bySetting[cacheRoomId] = newValue; bySetting[cacheRoomId] = newValue;
const currentValue = this.handler.getValue(settingName, roomId); const currentValue = this.handler.getValue(settingName, roomId);
const handlerPromise = this.handler.setValue(settingName, roomId, newValue); const handlerPromise = this.handler.setValue(settingName, roomId, newValue);
this.handler.watchers?.notifyUpdate(settingName, roomId, this.level, newValue); this.handler.watchers?.notifyUpdate(settingName, roomId, this.level, newValue);
return Promise.resolve(handlerPromise).catch(() => {
try {
await handlerPromise;
} catch (e) {
// notify of a rollback // notify of a rollback
this.handler.watchers?.notifyUpdate(settingName, roomId, this.level, currentValue); this.handler.watchers?.notifyUpdate(settingName, roomId, this.level, currentValue);
}).finally(() => { } finally {
delete bySetting[cacheRoomId]; // only expire the cache if our value hasn't been overwritten yet
}); if (bySetting[cacheRoomId] === newValue) {
delete bySetting[cacheRoomId];
}
}
} }
public canSetValue(settingName: string, roomId: string): boolean { public canSetValue(settingName: string, roomId: string): boolean {

View File

@ -15,7 +15,6 @@ limitations under the License.
*/ */
import { MatrixClient } from "matrix-js-sdk/src/client"; import { MatrixClient } from "matrix-js-sdk/src/client";
import { logger } from "matrix-js-sdk/src/logger";
import SettingsHandler from "./SettingsHandler"; import SettingsHandler from "./SettingsHandler";
@ -49,7 +48,5 @@ export default abstract class MatrixClientBackedSettingsHandler extends Settings
return MatrixClientBackedSettingsHandler._matrixClient; return MatrixClientBackedSettingsHandler._matrixClient;
} }
protected initMatrixClient(oldClient: MatrixClient, newClient: MatrixClient) { protected abstract initMatrixClient(oldClient: MatrixClient, newClient: MatrixClient);
logger.warn("initMatrixClient not overridden");
}
} }

View File

@ -18,14 +18,15 @@ limitations under the License.
import { MatrixClient } from "matrix-js-sdk/src/client"; import { MatrixClient } from "matrix-js-sdk/src/client";
import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { MatrixEvent } from "matrix-js-sdk/src/models/event";
import { Room, RoomEvent } from "matrix-js-sdk/src/models/room"; import { Room, RoomEvent } from "matrix-js-sdk/src/models/room";
import { defer } from "matrix-js-sdk/src/utils";
import { MatrixClientPeg } from '../../MatrixClientPeg';
import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler"; import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler";
import { objectClone, objectKeyChanges } from "../../utils/objects"; import { objectClone, objectKeyChanges } from "../../utils/objects";
import { SettingLevel } from "../SettingLevel"; import { SettingLevel } from "../SettingLevel";
import { WatchManager } from "../WatchManager"; import { WatchManager } from "../WatchManager";
const ALLOWED_WIDGETS_EVENT_TYPE = "im.vector.setting.allowed_widgets"; const ALLOWED_WIDGETS_EVENT_TYPE = "im.vector.setting.allowed_widgets";
const DEFAULT_SETTINGS_EVENT_TYPE = "im.vector.web.settings";
/** /**
* Gets and sets settings at the "room-account" level for the current user. * Gets and sets settings at the "room-account" level for the current user.
@ -55,7 +56,7 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin
} }
this.watchers.notifyUpdate("urlPreviewsEnabled", roomId, SettingLevel.ROOM_ACCOUNT, val); this.watchers.notifyUpdate("urlPreviewsEnabled", roomId, SettingLevel.ROOM_ACCOUNT, val);
} else if (event.getType() === "im.vector.web.settings") { } else if (event.getType() === DEFAULT_SETTINGS_EVENT_TYPE) {
// Figure out what changed and fire those updates // Figure out what changed and fire those updates
const prevContent = prevEvent ? prevEvent.getContent() : {}; const prevContent = prevEvent ? prevEvent.getContent() : {};
const changedSettings = objectKeyChanges<Record<string, any>>(prevContent, event.getContent()); const changedSettings = objectKeyChanges<Record<string, any>>(prevContent, event.getContent());
@ -87,43 +88,62 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin
return settings[settingName]; return settings[settingName];
} }
public async setValue(settingName: string, roomId: string, newValue: any): Promise<void> { // helper function to send room account data then await it being echoed back
// Special case URL previews private async setRoomAccountData(
if (settingName === "urlPreviewsEnabled") { roomId: string,
const content = this.getSettings(roomId, "org.matrix.room.preview_urls") || {}; eventType: string,
content['disable'] = !newValue; field: string | null,
await MatrixClientPeg.get().setRoomAccountData(roomId, "org.matrix.room.preview_urls", content); value: any,
return; ): Promise<void> {
let content: ReturnType<RoomAccountSettingsHandler["getSettings"]>;
if (field === null) {
content = value;
} else {
const content = this.getSettings(roomId, eventType) || {};
content[field] = value;
} }
// Special case allowed widgets await this.client.setRoomAccountData(roomId, eventType, content);
if (settingName === "allowedWidgets") {
await MatrixClientPeg.get().setRoomAccountData(roomId, ALLOWED_WIDGETS_EVENT_TYPE, newValue);
return;
}
const content = this.getSettings(roomId) || {}; const deferred = defer<void>();
content[settingName] = newValue; const handler = (event: MatrixEvent) => {
await MatrixClientPeg.get().setRoomAccountData(roomId, "im.vector.web.settings", content); if (event.getRoomId() !== roomId || event.getType() !== eventType) return;
if (field !== null && event.getContent()[field] !== value) return;
this.client.off(RoomEvent.AccountData, handler);
deferred.resolve();
};
this.client.on(RoomEvent.AccountData, handler);
await deferred.promise;
}
public setValue(settingName: string, roomId: string, newValue: any): Promise<void> {
switch (settingName) {
// Special case URL previews
case "urlPreviewsEnabled":
return this.setRoomAccountData(roomId, "org.matrix.room.preview_urls", "disable", !newValue);
// Special case allowed widgets
case "allowedWidgets":
return this.setRoomAccountData(roomId, ALLOWED_WIDGETS_EVENT_TYPE, null, newValue);
default:
return this.setRoomAccountData(roomId, DEFAULT_SETTINGS_EVENT_TYPE, settingName, newValue);
}
} }
public canSetValue(settingName: string, roomId: string): boolean { public canSetValue(settingName: string, roomId: string): boolean {
const room = MatrixClientPeg.get().getRoom(roomId);
// If they have the room, they can set their own account data // If they have the room, they can set their own account data
return room !== undefined && room !== null; return !!this.client.getRoom(roomId);
} }
public isSupported(): boolean { public isSupported(): boolean {
const cli = MatrixClientPeg.get(); return this.client && !this.client.isGuest();
return cli !== undefined && cli !== null && !cli.isGuest();
} }
private getSettings(roomId: string, eventType = "im.vector.web.settings"): any { // TODO: [TS] Type return private getSettings(roomId: string, eventType = DEFAULT_SETTINGS_EVENT_TYPE): any { // TODO: [TS] Type return
const room = MatrixClientPeg.get().getRoom(roomId); const event = this.client.getRoom(roomId)?.getAccountData(eventType);
if (!room) return null;
const event = room.getAccountData(eventType);
if (!event || !event.getContent()) return null; if (!event || !event.getContent()) return null;
return objectClone(event.getContent()); // clone to prevent mutation return objectClone(event.getContent()); // clone to prevent mutation
} }

View File

@ -18,13 +18,15 @@ limitations under the License.
import { MatrixClient } from "matrix-js-sdk/src/client"; import { MatrixClient } from "matrix-js-sdk/src/client";
import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { MatrixEvent } from "matrix-js-sdk/src/models/event";
import { RoomState, RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; import { RoomState, RoomStateEvent } from "matrix-js-sdk/src/models/room-state";
import { defer } from "matrix-js-sdk/src/utils";
import { MatrixClientPeg } from '../../MatrixClientPeg';
import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler"; import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler";
import { objectClone, objectKeyChanges } from "../../utils/objects"; import { objectClone, objectKeyChanges } from "../../utils/objects";
import { SettingLevel } from "../SettingLevel"; import { SettingLevel } from "../SettingLevel";
import { WatchManager } from "../WatchManager"; import { WatchManager } from "../WatchManager";
const DEFAULT_SETTINGS_EVENT_TYPE = "im.vector.web.settings";
/** /**
* Gets and sets settings at the "room" level. * Gets and sets settings at the "room" level.
*/ */
@ -63,13 +65,12 @@ export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandl
} }
this.watchers.notifyUpdate("urlPreviewsEnabled", roomId, SettingLevel.ROOM, val); this.watchers.notifyUpdate("urlPreviewsEnabled", roomId, SettingLevel.ROOM, val);
} else if (event.getType() === "im.vector.web.settings") { } else if (event.getType() === DEFAULT_SETTINGS_EVENT_TYPE) {
// Figure out what changed and fire those updates // Figure out what changed and fire those updates
const prevContent = prevEvent ? prevEvent.getContent() : {}; const prevContent = prevEvent ? prevEvent.getContent() : {};
const changedSettings = objectKeyChanges<Record<string, any>>(prevContent, event.getContent()); const changedSettings = objectKeyChanges<Record<string, any>>(prevContent, event.getContent());
for (const settingName of changedSettings) { for (const settingName of changedSettings) {
this.watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM, this.watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM, event.getContent()[settingName]);
event.getContent()[settingName]);
} }
} }
}; };
@ -88,42 +89,56 @@ export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandl
return settings[settingName]; return settings[settingName];
} }
public async setValue(settingName: string, roomId: string, newValue: any): Promise<void> { // helper function to send state event then await it being echoed back
// Special case URL previews private async sendStateEvent(
if (settingName === "urlPreviewsEnabled") { roomId: string,
const content = this.getSettings(roomId, "org.matrix.room.preview_urls") || {}; eventType: string,
content['disable'] = !newValue; field: string,
await MatrixClientPeg.get().sendStateEvent(roomId, "org.matrix.room.preview_urls", content); value: any,
return; ): Promise<void> {
} const content = this.getSettings(roomId, eventType) || {};
content[field] = value;
const content = this.getSettings(roomId) || {}; const { event_id: eventId } = await this.client.sendStateEvent(roomId, eventType, content);
content[settingName] = newValue;
await MatrixClientPeg.get().sendStateEvent(roomId, "im.vector.web.settings", content, ""); const deferred = defer<void>();
const handler = (event: MatrixEvent) => {
if (event.getId() !== eventId) return;
this.client.off(RoomStateEvent.Events, handler);
deferred.resolve();
};
this.client.on(RoomStateEvent.Events, handler);
await deferred.promise;
}
public setValue(settingName: string, roomId: string, newValue: any): Promise<void> {
switch (settingName) {
// Special case URL previews
case "urlPreviewsEnabled":
return this.sendStateEvent(roomId, "org.matrix.room.preview_urls", "disable", !newValue);
default:
return this.sendStateEvent(roomId, DEFAULT_SETTINGS_EVENT_TYPE, settingName, newValue);
}
} }
public canSetValue(settingName: string, roomId: string): boolean { public canSetValue(settingName: string, roomId: string): boolean {
const cli = MatrixClientPeg.get(); const room = this.client.getRoom(roomId);
const room = cli.getRoom(roomId);
let eventType = "im.vector.web.settings"; let eventType = DEFAULT_SETTINGS_EVENT_TYPE;
if (settingName === "urlPreviewsEnabled") eventType = "org.matrix.room.preview_urls"; if (settingName === "urlPreviewsEnabled") eventType = "org.matrix.room.preview_urls";
if (!room) return false; return room?.currentState.maySendStateEvent(eventType, this.client.getUserId()) ?? false;
return room.currentState.maySendStateEvent(eventType, cli.getUserId());
} }
public isSupported(): boolean { public isSupported(): boolean {
const cli = MatrixClientPeg.get(); return !!this.client;
return cli !== undefined && cli !== null;
} }
private getSettings(roomId: string, eventType = "im.vector.web.settings"): any { private getSettings(roomId: string, eventType = DEFAULT_SETTINGS_EVENT_TYPE): any {
const room = MatrixClientPeg.get().getRoom(roomId); const event = this.client.getRoom(roomId)?.currentState.getStateEvents(eventType, "");
if (!room) return null; if (!event?.getContent()) return null;
const event = room.currentState.getStateEvents(eventType, "");
if (!event || !event.getContent()) return null;
return objectClone(event.getContent()); // clone to prevent mutation return objectClone(event.getContent()); // clone to prevent mutation
} }
} }

View File

@ -20,6 +20,7 @@ import dis from '../../src/dispatcher/dispatcher';
import { makeType } from "../../src/utils/TypeUtils"; import { makeType } from "../../src/utils/TypeUtils";
import { ValidatedServerConfig } from "../../src/utils/AutoDiscoveryUtils"; import { ValidatedServerConfig } from "../../src/utils/AutoDiscoveryUtils";
import { EnhancedMap } from "../../src/utils/maps"; import { EnhancedMap } from "../../src/utils/maps";
import MatrixClientBackedSettingsHandler from "../../src/settings/handlers/MatrixClientBackedSettingsHandler";
/** /**
* Stub out the MatrixClient, and configure the MatrixClientPeg object to * Stub out the MatrixClient, and configure the MatrixClientPeg object to
@ -44,6 +45,7 @@ export function stubClient() {
// MatrixClientPeg.get() is called a /lot/, so implement it with our own // MatrixClientPeg.get() is called a /lot/, so implement it with our own
// fast stub function rather than a sinon stub // fast stub function rather than a sinon stub
peg.get = function() { return client; }; peg.get = function() { return client; };
MatrixClientBackedSettingsHandler.matrixClient = client;
} }
/** /**