From 3059b5b1e2154534d7185e6bd3075c373488f6b3 Mon Sep 17 00:00:00 2001
From: Travis Ralston <travisr@matrix.org>
Date: Fri, 3 May 2024 00:31:34 -0600
Subject: [PATCH] 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 https://github.com/element-hq/element-web/issues/27426

* Appease the linter

* Reset for each test
---
 src/SlidingSyncManager.ts       |  6 ++++-
 test/SlidingSyncManager-test.ts | 48 ++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/src/SlidingSyncManager.ts b/src/SlidingSyncManager.ts
index e3f420b43d5..6f118d9b460 100644
--- a/src/SlidingSyncManager.ts
+++ b/src/SlidingSyncManager.ts
@@ -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",
             });
diff --git a/test/SlidingSyncManager-test.ts b/test/SlidingSyncManager-test.ts
index 757a682d848..bca00fd1454 100644
--- a/test/SlidingSyncManager-test.ts
+++ b/test/SlidingSyncManager-test.ts
@@ -16,7 +16,8 @@ limitations under the License.
 
 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";
@@ -253,6 +254,51 @@ describe("SlidingSyncManager", () => {
             expect(SlidingSyncController.serverSupportsSlidingSync).toBeTruthy();
         });
     });
+    describe("nativeSlidingSyncSupport", () => {
+        beforeEach(() => {
+            SlidingSyncController.serverSupportsSlidingSync = false;
+        });
+        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();
+
+            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");