Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Use OPTIONS for sliding sync detection poke #12492

Merged
merged 4 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/SlidingSyncManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,11 @@ export class SlidingSyncManager {
*/
public async nativeSlidingSyncSupport(client: MatrixClient): Promise<boolean> {
try {
await client.http.authedRequest<void>(Method.Post, "/sync", undefined, undefined, {
// We use OPTIONS to avoid causing a real sync to happen, as that may be intensive or encourage
// middleware software to start polling as our access token (thus stealing our to-device messages).
// See https://github.com/element-hq/element-web/issues/27426
// XXX: Using client.http is a bad thing - it's meant to be private access. See `client.http` for details.
await client.http.authedRequest<void>(Method.Options, "/sync", undefined, undefined, {
localTimeoutMs: 10 * 1000, // 10s
prefix: "/_matrix/client/unstable/org.matrix.msc3575",
});
Expand Down
45 changes: 44 additions & 1 deletion test/SlidingSyncManager-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@

import { SlidingSync } from "matrix-js-sdk/src/sliding-sync";
import { mocked } from "jest-mock";
import { MatrixClient, MatrixEvent, Room } from "matrix-js-sdk/src/matrix";
import { IRequestOpts, MatrixClient, MatrixEvent, Method, Room } from "matrix-js-sdk/src/matrix";
import { QueryDict } from "matrix-js-sdk/src/utils";

import { SlidingSyncManager } from "../src/SlidingSyncManager";
import { stubClient } from "./test-utils";
Expand Down Expand Up @@ -253,6 +254,48 @@
expect(SlidingSyncController.serverSupportsSlidingSync).toBeTruthy();
});
});
describe("nativeSlidingSyncSupport", () => {
it("should make an OPTIONS request to avoid unintended side effects", async () => {
// See https://github.com/element-hq/element-web/issues/27426

// Developer note: We mock this in a truly terrible way because of how the call is done. There's not
// really much we can do to avoid it.
client.http = {
async authedRequest(
method: Method,
path: string,
queryParams?: QueryDict,
body?: Body,
paramOpts: IRequestOpts & {
doNotAttemptTokenRefresh?: boolean;
} = {},
): Promise<any> {
// XXX: Ideally we'd use ResponseType<> like in the real thing, but it's not exported
expect(method).toBe(Method.Options);
expect(path).toBe("/sync");
expect(queryParams).toBeUndefined();
expect(body).toBeUndefined();
expect(paramOpts).toEqual({
localTimeoutMs: 10 * 1000, // 10s
prefix: "/_matrix/client/unstable/org.matrix.msc3575",
});
return {};
},
} as any;

const proxySpy = jest.spyOn(manager, "getProxyFromWellKnown").mockResolvedValue("proxy");

expect(SlidingSyncController.serverSupportsSlidingSync).toBeFalsy();

Check failure on line 288 in test/SlidingSyncManager-test.ts

View workflow job for this annotation

GitHub Actions / Jest (2)

SlidingSyncManager › nativeSlidingSyncSupport › should make an OPTIONS request to avoid unintended side effects

expect(received).toBeFalsy() Received: true at Object.toBeFalsy (test/SlidingSyncManager-test.ts:288:69)

await manager.checkSupport(client); // first thing it does is call nativeSlidingSyncSupport

// Note: if this expectation is failing, it may mean the authedRequest mock threw an expectation failure
// which got consumed by `nativeSlidingSyncSupport`. Log your errors to discover more.
expect(proxySpy).not.toHaveBeenCalled();

expect(SlidingSyncController.serverSupportsSlidingSync).toBeTruthy();
});
});
describe("setup", () => {
beforeEach(() => {
jest.spyOn(manager, "configure");
Expand Down
Loading