Skip to content

Commit

Permalink
Use OPTIONS for sliding sync detection poke (#12492)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
turt2live committed May 3, 2024
1 parent d25d529 commit 3059b5b
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 2 deletions.
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
48 changes: 47 additions & 1 deletion test/SlidingSyncManager-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit 3059b5b

Please sign in to comment.