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

Commit 3059b5b

Browse files
authored
Use OPTIONS for sliding sync detection poke (#12492)
* Use OPTIONS for sliding sync detection poke This avoids unintended consequences, including high resource usage, which would accompany a "full" sync request. Instead, we just grab headers and enough information for CORS to pass, revealing likely support. Fixes element-hq/element-web#27426 * Appease the linter * Reset for each test
1 parent d25d529 commit 3059b5b

File tree

2 files changed

+52
-2
lines changed

2 files changed

+52
-2
lines changed

src/SlidingSyncManager.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,11 @@ export class SlidingSyncManager {
378378
*/
379379
public async nativeSlidingSyncSupport(client: MatrixClient): Promise<boolean> {
380380
try {
381-
await client.http.authedRequest<void>(Method.Post, "/sync", undefined, undefined, {
381+
// We use OPTIONS to avoid causing a real sync to happen, as that may be intensive or encourage
382+
// middleware software to start polling as our access token (thus stealing our to-device messages).
383+
// See https://github.com/element-hq/element-web/issues/27426
384+
// XXX: Using client.http is a bad thing - it's meant to be private access. See `client.http` for details.
385+
await client.http.authedRequest<void>(Method.Options, "/sync", undefined, undefined, {
382386
localTimeoutMs: 10 * 1000, // 10s
383387
prefix: "/_matrix/client/unstable/org.matrix.msc3575",
384388
});

test/SlidingSyncManager-test.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ limitations under the License.
1616

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

2122
import { SlidingSyncManager } from "../src/SlidingSyncManager";
2223
import { stubClient } from "./test-utils";
@@ -253,6 +254,51 @@ describe("SlidingSyncManager", () => {
253254
expect(SlidingSyncController.serverSupportsSlidingSync).toBeTruthy();
254255
});
255256
});
257+
describe("nativeSlidingSyncSupport", () => {
258+
beforeEach(() => {
259+
SlidingSyncController.serverSupportsSlidingSync = false;
260+
});
261+
it("should make an OPTIONS request to avoid unintended side effects", async () => {
262+
// See https://github.com/element-hq/element-web/issues/27426
263+
264+
// Developer note: We mock this in a truly terrible way because of how the call is done. There's not
265+
// really much we can do to avoid it.
266+
client.http = {
267+
async authedRequest(
268+
method: Method,
269+
path: string,
270+
queryParams?: QueryDict,
271+
body?: Body,
272+
paramOpts: IRequestOpts & {
273+
doNotAttemptTokenRefresh?: boolean;
274+
} = {},
275+
): Promise<any> {
276+
// XXX: Ideally we'd use ResponseType<> like in the real thing, but it's not exported
277+
expect(method).toBe(Method.Options);
278+
expect(path).toBe("/sync");
279+
expect(queryParams).toBeUndefined();
280+
expect(body).toBeUndefined();
281+
expect(paramOpts).toEqual({
282+
localTimeoutMs: 10 * 1000, // 10s
283+
prefix: "/_matrix/client/unstable/org.matrix.msc3575",
284+
});
285+
return {};
286+
},
287+
} as any;
288+
289+
const proxySpy = jest.spyOn(manager, "getProxyFromWellKnown").mockResolvedValue("proxy");
290+
291+
expect(SlidingSyncController.serverSupportsSlidingSync).toBeFalsy();
292+
293+
await manager.checkSupport(client); // first thing it does is call nativeSlidingSyncSupport
294+
295+
// Note: if this expectation is failing, it may mean the authedRequest mock threw an expectation failure
296+
// which got consumed by `nativeSlidingSyncSupport`. Log your errors to discover more.
297+
expect(proxySpy).not.toHaveBeenCalled();
298+
299+
expect(SlidingSyncController.serverSupportsSlidingSync).toBeTruthy();
300+
});
301+
});
256302
describe("setup", () => {
257303
beforeEach(() => {
258304
jest.spyOn(manager, "configure");

0 commit comments

Comments
 (0)