Skip to content

Commit

Permalink
Do not rely on age, use origin_server_ts (#842)
Browse files Browse the repository at this point in the history
* Never use age, it's not reliable

* Fix types

* changelog

* Update tests

* Further unsigned fixes
  • Loading branch information
Half-Shot authored Sep 12, 2022
1 parent 942f912 commit ac5180e
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 21 deletions.
1 change: 1 addition & 0 deletions changelog.d/842.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove usage of unreliable field `age` on events, allowing the bridge to work with non-Synapse homeserver implementations.
12 changes: 5 additions & 7 deletions src/matrixeventprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,9 @@ export class MatrixEventProcessor {
*/
public async OnEvent(event: IMatrixEvent, rooms: IRoomStoreEntry[]): Promise<void> {
const remoteRoom = rooms[0];
if (event.unsigned.age > AGE_LIMIT) {
log.info(`Skipping event due to age ${event.unsigned.age} > ${AGE_LIMIT}`);
// throw new Unstable.EventTooOldError(
// `Skipping event due to age ${event.unsigned.age} > ${AGE_LIMIT}`,
// );
const age = Date.now() - event.origin_server_ts;
if (age > AGE_LIMIT) {
log.info(`Skipping event due to age ${age} > ${AGE_LIMIT}`);
return;
}
if (
Expand Down Expand Up @@ -247,8 +245,8 @@ export class MatrixEventProcessor {
} else if (event.type === "m.room.member") {
const membership = event.content!.membership;
const client = this.bridge.botIntent.underlyingClient;
const isNewJoin = event.unsigned.replaces_state === undefined ? true : (
await client.getEvent(event.room_id, event.unsigned.replaces_state)).content.membership !== "join";
const isNewJoin = event.unsigned?.replaces_state === undefined ? true : (
await client.getEvent(event.room_id, event.unsigned?.replaces_state)).content.membership !== "join";
if (membership === "join") {
this.mxUserProfileCache.delete(`${event.room_id}:${event.sender}`);
this.mxUserProfileCache.delete(event.sender);
Expand Down
6 changes: 4 additions & 2 deletions src/matrixtypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ export interface IMatrixEvent {
redacts?: string;
replaces_state?: string;
content?: IMatrixEventContent;
unsigned?: any; // tslint:disable-line no-any
origin_server_ts?: number;
origin_server_ts: number;
users?: any; // tslint:disable-line no-any
users_default?: any; // tslint:disable-line no-any
notifications?: any; // tslint:disable-line no-any
unsigned?: {
replaces_state: any; // tslint:disable-line no-any
}
}

export interface IMatrixMessage {
Expand Down
16 changes: 4 additions & 12 deletions test/test_matrixeventprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,19 @@ import { MockChannel } from "./mocks/channel";
import { IMatrixEvent } from "../src/matrixtypes";
import { AppserviceMock } from "./mocks/appservicemock";
import { Appservice } from "matrix-bot-sdk";
import { RemoteStoreRoom } from "../src/db/roomstore";

// we are a test file and thus need those
/* tslint:disable:no-unused-expression max-file-line-count no-any */

const TEST_TIMESTAMP = 1337;

function buildRequest(eventData): IMatrixEvent {
if (eventData.unsigned === undefined) {
eventData.unsigned = {age: 0};
}
if (eventData.sender === undefined) {
eventData.sender = "@foobar:localhost";
}
if (!eventData.origin_server_ts) {
eventData.origin_server_ts = Date.now();
}
return eventData;
}

Expand Down Expand Up @@ -378,7 +377,6 @@ describe("MatrixEventProcessor", () => {
},
sender: "@user:localhost",
type: "m.room.member",
unsigned: {},
} as IMatrixEvent;
await processor.ProcessStateEvent(event);
expect(STATE_EVENT_MSG).to.equal("`@user:localhost` joined the room on Matrix.");
Expand Down Expand Up @@ -407,7 +405,6 @@ describe("MatrixEventProcessor", () => {
sender: "@user:localhost",
state_key: "@user2:localhost",
type: "m.room.member",
unsigned: {},
} as IMatrixEvent;
await processor.ProcessStateEvent(event);
expect(STATE_EVENT_MSG).to.equal("`@user:localhost` invited `@user2:localhost` to the room on Matrix.");
Expand Down Expand Up @@ -437,7 +434,6 @@ describe("MatrixEventProcessor", () => {
sender: "@user:localhost",
state_key: "@user2:localhost",
type: "m.room.member",
unsigned: {},
} as IMatrixEvent;
await processor.ProcessStateEvent(event);
expect(STATE_EVENT_MSG).to.equal("`@user:localhost` kicked `@user2:localhost` from the room on Matrix.");
Expand All @@ -451,7 +447,6 @@ describe("MatrixEventProcessor", () => {
sender: "@user:localhost",
state_key: "@user:localhost",
type: "m.room.member",
unsigned: {},
} as IMatrixEvent;
await processor.ProcessStateEvent(event);
expect(STATE_EVENT_MSG).to.equal("`@user:localhost` left the room on Matrix.");
Expand Down Expand Up @@ -481,7 +476,6 @@ describe("MatrixEventProcessor", () => {
sender: "@user:localhost",
state_key: "@user2:localhost",
type: "m.room.member",
unsigned: {},
} as IMatrixEvent;
await processor.ProcessStateEvent(event);
expect(STATE_EVENT_MSG).to.equal("`@user:localhost` banned `@user2:localhost` from the room on Matrix.");
Expand Down Expand Up @@ -971,21 +965,19 @@ This is the reply`,
const {processor} = createMatrixEventProcessor();
let err;
try {
await processor.OnEvent(buildRequest({unsigned: {age: AGE}}), []);
await processor.OnEvent(buildRequest({ origin_server_ts: Date.now() - AGE }), []);
} catch (e) { err = e; }
// TODO: Not supported yet.
// expect(err).to.be.an.instanceof(Unstable.EventTooOldError);
});
it("should reject un-processable events", async () => {
const AGE = 900000; // 15 * 60 * 1000
const {processor} = createMatrixEventProcessor();
let err;
try {
await processor.OnEvent(
buildRequest({
content: {},
type: "m.potato",
unsigned: {age: AGE},
}),
[],
);
Expand Down

0 comments on commit ac5180e

Please sign in to comment.