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

Fix empty string to room compatibility trick to only apply to m.call (#5172)

Browse Source 
* Fix empty string to room compatibility trick to only apply to m.call

* add logging

* fix linter

* Add tests

* limit logging.
This commit is contained in:
Timo
2026-02-08 12:25:34 +01:00
committed by GitHub
parent fb590627bb
commit 6e3efef0c5
4 changed files with 114 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spec/unit/matrixrtc/CallMembership.spec.ts
+25
View File
@@ -158,10 +158,35 @@ describe("CallMembership", () => {
expect(membership.eventId).toBe("$eventid");
});
it("returns correct slot_id", () => {
// slot_id is application and call_id dependent. So we create
// a membership for each possible combination
// non call application (should not alter call_id even with empty string)
const nonCallMembership = createCallMembership(makeMockEvent(), {
...membershipTemplate,
application: "m.not.a.call",
call_id: "",
});
// non "" call id should not be altered
const callMembershipCustomId = createCallMembership(makeMockEvent(), {
...membershipTemplate,
call_id: "customCallId",
});
// for membership (application = m.call and call_id = "") we expect "" -> ROOM
// for legacy events we expect the room to be added automagically
// See INFO_SLOT_ID_LEGACY_CASE comments
expect(membership.slotId).toBe("m.call#ROOM");
expect(membership.slotDescription).toStrictEqual({ id: "ROOM", application: "m.call" });
expect(nonCallMembership.slotId).toBe("m.not.a.call#");
expect(nonCallMembership.slotDescription).toStrictEqual({ id: "", application: "m.not.a.call" });
expect(callMembershipCustomId.slotId).toBe("m.call#customCallId");
expect(callMembershipCustomId.slotDescription).toStrictEqual({
id: "customCallId",
application: "m.call",
});
});
it("returns correct deviceId", () => {
expect(membership.deviceId).toBe("AAAAAAA");
spec/unit/matrixrtc/MembershipManager.spec.ts
+41
View File
@@ -139,6 +139,7 @@ describe("MembershipManager", () => {
"org.matrix.msc3401.call.member",
{
application: "m.call",
// This tests INFO_SLOT_ID_LEGACY_CASE because it is using callSession = { id: "ROOM", application: "m.call" }
call_id: "",
device_id: "AAAAAAA",
expires: 14400000,
@@ -147,6 +148,7 @@ describe("MembershipManager", () => {
focus_active: focusActive,
scope: "m.room",
},
// This tests INFO_SLOT_ID_LEGACY_CASE because it is using callSession = { id: "ROOM", application: "m.call" }
"_@alice:example.org_AAAAAAA_m.call",
);
restartScheduledDelayedEventHandle.resolve?.();
@@ -160,6 +162,45 @@ describe("MembershipManager", () => {
expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledTimes(1);
});
it("sends correct call_id and state key when using non empty string. Not using empty string -> ROOM hack. See: INFO_SLOT_ID_LEGACY_CASE", async () => {
// Spys/Mocks
const customCallSession = { id: "custom", application: "m.call" };
const restartScheduledDelayedEventHandle = createAsyncHandle<void>(
client._unstable_restartScheduledDelayedEvent,
);
// Test
const memberManager = new MembershipManager(undefined, room, client, customCallSession);
memberManager.join([focus], undefined);
// expects
await waitForMockCall(client.sendStateEvent, Promise.resolve({ event_id: "id" }));
expect(client.sendStateEvent).toHaveBeenCalledWith(
room.roomId,
"org.matrix.msc3401.call.member",
{
application: "m.call",
call_id: "custom",
device_id: "AAAAAAA",
expires: 14400000,
foci_preferred: [focus],
membershipID: "@alice:example.org:AAAAAAA",
focus_active: focusActive,
scope: "m.room",
},
"_@alice:example.org_AAAAAAA_m.callcustom",
);
restartScheduledDelayedEventHandle.resolve?.();
expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledWith(
room.roomId,
{ delay: 8000 },
"org.matrix.msc3401.call.member",
{},
"_@alice:example.org_AAAAAAA_m.callcustom",
);
expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledTimes(1);
});
it("reschedules delayed leave event if sending state cancels it", async () => {
const memberManager = new MembershipManager(undefined, room, client, callSession);
const waitForSendState = waitForMockCall(client.sendStateEvent);
src/matrixrtc/CallMembership.ts
+43 -18
View File
@@ -372,28 +372,53 @@ export class CallMembership {
*/
public get slotId(): string {
const { kind, data } = this.membershipData;
if (data.application === "m.call") {
switch (kind) {
case "rtc":
return data.slot_id;
case "session":
default: {
const [application, id] = [this.application, data.call_id];
// INFO_SLOT_ID_LEGACY_CASE (search for all occurances of this INFO to get the full picture)
// The spec got changed to use `"ROOM"` instead of `""` empyt string for the implicit default call.
// State events still are sent with `""` however. To find other events that should end up in the same call,
// we use the slotId.
// Since the CallMembership is the public representation of a rtc.member event, we just pretend it is a
// "ROOM" slotId/call_id.
// This makes all the remote members work with just this simple trick.
//
// We of course now need to be careful when sending legacy events (state events)
// They get a slotDescription containing "ROOM" since this is what we use starting at the time this comment
// is commited.
//
// See the Other INFO_SLOT_ID_LEGACY_CASE comments to see where we revert back to "" just before sending the event.
let compatibilityAdaptedId: string;
if (id === "") {
compatibilityAdaptedId = "ROOM";
this.logger?.info("use slotId compat hack emptyString -> ROOM");
} else {
compatibilityAdaptedId = id;
}
return slotDescriptionToId({
application,
id: compatibilityAdaptedId,
});
}
}
}
this.logger?.info("NOT using slotId compat hack emptyString -> ROOM");
// This is what the function should look like for any other application that did not
// go through a `""`=> `"ROOM"` rename
switch (kind) {
case "rtc":
return data.slot_id;
case "session":
default:
// INFO_SLOT_ID_LEGACY_CASE (search for all occurances of this INFO to get the full picture)
// The spec got changed to use `"ROOM"` instead of `""` empyt string for the implicit default call.
// State events still are sent with `""` however. To find other events that should end up in the same call,
// we use the slotId.
// Since the CallMembership is the public representation of a rtc.member event, we just pretend it is a
// "ROOM" slotId/call_id.
// This makes all the remote members work with just this simple trick.
//
// We of course now need to be careful when sending legacy events (state events)
// They get a slotDescription containing "ROOM" since this is what we use starting at the time this comment
// is commited.
//
// See the Other INFO_SLOT_ID_LEGACY_CASE comments to see where we revert back to "" just before sending the event.
return slotDescriptionToId({
application: this.application,
id: data.call_id === "" ? "ROOM" : data.call_id,
});
default: {
const [application, id] = [this.application, data.call_id];
return slotDescriptionToId({ application, id });
}
}
}
src/matrixrtc/MembershipManager.ts
+5 -2
View File
@@ -777,7 +777,8 @@ export class MembershipManager
// INFO_SLOT_ID_LEGACY_CASE (search for all occurances of this INFO to get the full picture)
// Revert back to "" just for the state key (state keys are always legacy. we use sticky events for non legacy events)
const application = this.slotDescription.application;
const slotId = this.slotDescription.id === "ROOM" ? "" : this.slotDescription.id;
const needsEmptyStringRoomFix = application === "m.call" && this.slotDescription.id === "ROOM";
const slotId = needsEmptyStringRoomFix ? "" : this.slotDescription.id;
const stateKey = `${localUserId}_${localDeviceId}_${application}${slotId}`;
if (/^org\.matrix\.msc(3757|3779)\b/.exec(this.room.getVersion())) {
return stateKey;
@@ -791,6 +792,8 @@ export class MembershipManager
*/
protected makeMyMembership(expires: number): SessionMembershipData | RtcMembershipData {
const ownMembership = this.ownMembership;
const needsEmptyStringRoomFix =
this.slotDescription.application === "m.call" && this.slotDescription.id === "ROOM";
const focusObjects =
this.rtcTransport === undefined
@@ -806,7 +809,7 @@ export class MembershipManager
"application": this.slotDescription.application,
// INFO_SLOT_ID_LEGACY_CASE (search for all occurances of this INFO to get the full picture)
// Revert back to "" just for the sending the event.
"call_id": this.slotDescription.id === "ROOM" ? "" : this.slotDescription.id,
"call_id": needsEmptyStringRoomFix ? "" : this.slotDescription.id,
"scope": "m.room",
"device_id": this.deviceId,
// DO NOT use this.memberId here since that is the state key (using application...)