From dfba7486e22a8bd8e8eac0cc130c1170e1b0c8f2 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Wed, 26 Aug 2026 21:13:49 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(collaboration)=20fall=20back=20to=20h?= =?UTF-8?q?ttp=20polling=20when=20the=20websocket=20is=20blocked?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some networks refuse a websocket upgrade - corporate proxies, captive portals - and a browser is told nothing more than "the connection closed", so those users could not edit at all. The editor now runs a second transport next to the socket, polling the collaboration server's REST api on the same room, with the same session cookie and the same authorization, and only while the socket is down. Local changes go out about a second after the last keystroke and remote ones arrive within ten seconds, so editing works with visibly more latency rather than not at all. The socket keeps being retried underneath, so a client that fell back during an outage returns to it on its own, and nothing is lost in either direction - both transports publish from the same document. This makes /collaboration/ydoc/ a route browsers call, so COLLABORATION_SERVER_ORIGIN is now handed to yhub as its cors configuration and gates the http routes as well as the websocket. Signed-off-by: Kevin Jahns --- CHANGELOG.md | 17 ++ documentation/collaboration.md | 30 +++- src/frontend/apps/e2e/.env | 2 +- src/frontend/apps/e2e/.env.example | 2 +- .../app-impress/doc-collaboration.spec.ts | 62 +++++++ src/frontend/apps/impress/package.json | 3 +- .../core/config/hooks/useCollaborationUrl.tsx | 26 +++ .../__tests__/useProviderStore.test.tsx | 136 +++++++++++++- .../stores/useProviderStore.tsx | 170 +++++++++++++----- src/frontend/yarn.lock | 43 ++++- src/yhub-server/README.md | 20 +++ src/yhub-server/package-lock.json | 12 +- src/yhub-server/package.json | 2 +- src/yhub-server/server.js | 23 ++- 14 files changed, 475 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 992cf7059..4a090079b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,23 @@ and this project adheres to ## [Unreleased] +### Added + +- ✨(frontend) fall back to http polling when the websocket cannot be opened. + Some networks refuse a websocket upgrade — corporate proxies, captive portals + — and a browser is told nothing more than "the connection closed"; those users + could not edit at all. The editor now runs a second transport next to the + socket, polling the collaboration server's REST api on the same room, with the + same session cookie and the same authorization, and only while the socket is + down. Local changes go out about a second after the last keystroke and remote + ones arrive within ten seconds, so editing works with visibly more latency + rather than not at all. The socket keeps being retried underneath, so a client + that fell back during an outage returns to it on its own, and nothing is lost + in either direction — both transports publish from the same document. This + makes `/collaboration/ydoc/` a route browsers call, so + `COLLABORATION_SERVER_ORIGIN` now gates the http routes as well as the + websocket + ### Fixed - 🐛(frontend) stop reconnecting to the collaboration server when it has refused diff --git a/documentation/collaboration.md b/documentation/collaboration.md index 68e8f35e5..3df7b70b4 100644 --- a/documentation/collaboration.md +++ b/documentation/collaboration.md @@ -62,5 +62,31 @@ Several replicas can serve the same document: they exchange updates through Redi ## What happens when connection to the websocket is not allowed? -When multiple users access a Docs and the connection to the websocket is not allowed, then they will be in a situation where they can lose data. -They will lose data because they will erase each other modifications. You can also have a scenario with a mix of users connected to the websocket and some other not. +Some networks refuse a websocket upgrade — corporate proxies, captive portals — and a browser is +told nothing more than "the connection closed". For those clients the editor falls back to polling +the collaboration server over plain http, on the same room, with the same session cookie and the +same authorization. Nothing has to be configured: the fallback is installed next to the websocket +and only ever sends a request while the socket is down. + +That means `/collaboration/ydoc/` has to be routed publicly, not only in-cluster — the browsers of +those users call it directly. And the origins a browser may reach the server from are the ones in +`COLLABORATION_SERVER_ORIGIN`, which now gate the http routes as well as the websocket: + +```yaml +COLLABORATION_SERVER_ORIGIN: https://{yourdocsdomain.tld} +``` + +A comma-separated list is allowed, and each entry is a bare origin — `https://host[:port]`, no path +and no trailing slash. A deployment serving the frontend from another origin than the collaboration +server has to list it here or the fallback is refused, the same way the websocket already is. + +What the fallback does *not* do is hide the difference. It publishes local changes about a second +after the last keystroke, and it retrieves the document every ten seconds, so someone else's edits +arrive with up to that much delay and remote cursors move at poll resolution. Each round transfers +the whole document, so a large document polled by many clients is real egress. It is a way to keep +editing, not a replacement for the socket — and the socket keeps being retried underneath, so a +client that fell back during an outage returns to it on its own. + +Documents are never in conflict either way: both transports publish from the same Yjs document, and +Yjs merges. Before the fallback existed, users who could not open a websocket edited a document that +was saved wholesale and erased each other's modifications; that is what this removes. diff --git a/src/frontend/apps/e2e/.env b/src/frontend/apps/e2e/.env index a0bd90c65..2d87c4b46 100644 --- a/src/frontend/apps/e2e/.env +++ b/src/frontend/apps/e2e/.env @@ -1,7 +1,7 @@ PORT=3000 BASE_URL=http://localhost:3000 BASE_API_URL=http://localhost:8071/api/v1.0 -COLLABORATION_WS_URL=ws://localhost:3002/ws/docs +COLLABORATION_WS_URL=ws://localhost:3002/collaboration/ws/v1/docs MEDIA_BASE_URL=http://localhost:8083 CUSTOM_SIGN_IN=false IS_INSTANCE=false diff --git a/src/frontend/apps/e2e/.env.example b/src/frontend/apps/e2e/.env.example index 45272cc11..dbd6802fd 100644 --- a/src/frontend/apps/e2e/.env.example +++ b/src/frontend/apps/e2e/.env.example @@ -1,7 +1,7 @@ PORT=3000 BASE_URL=http://localhost:3000 BASE_API_URL=http://localhost:8071/api/v1.0 -COLLABORATION_WS_URL=ws://localhost:3002/ws/docs +COLLABORATION_WS_URL=ws://localhost:3002/collaboration/ws/v1/docs MEDIA_BASE_URL=http://localhost:8083 IS_INSTANCE=false CUSTOM_SIGN_IN=false diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-collaboration.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-collaboration.spec.ts index fb237161a..816871ed0 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-collaboration.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-collaboration.spec.ts @@ -110,6 +110,68 @@ test.describe('Doc Collaboration', () => { await cleanup(); }); + /** + * The networks that refuse a websocket upgrade - corporate proxies, captive portals. + * `routeWebSocket` never forwards the connection to the server and closes it towards the + * page, which is what those look like from the browser: a socket that dies immediately, + * every time. The editor has to keep working over y/hub's REST api instead. + */ + test('falls back to http polling when the websocket cannot be opened', async ({ + page, + }) => { + // Polling is slower than a socket by construction, and this waits on it three times: the + // first retrieval, the publication about a second after the last keystroke, and the + // retrieval after the reload. The default 30s budget cannot cover that. + test.setTimeout(120000); + + await page.routeWebSocket(/\/collaboration\/ws\//, (ws) => ws.close()); + // the interception is injected when a document is created, so it only covers sockets + // opened after a navigation - `beforeEach` has already loaded this one + await page.goto('/'); + + const retrieved = page.waitForResponse( + (response) => + response.url().includes('/collaboration/ydoc/v1/') && + response.request().method() === 'GET', + { timeout: 45000 }, + ); + + await page + .getByRole('link', { + name: 'New', + exact: true, + }) + .click(); + + // a 401/403 here means the request is authorized differently than the websocket: + // the session cookie did not reach the collaboration server, or the origin was refused + expect((await retrieved).status()).toBe(200); + + // The one carrying the text, not merely the next one: awareness is published over the same + // route, so waiting for any PATCH would let the reload below race the document update, which + // is debounced until about a second after the last keystroke. Yjs stores inserted text as + // plain utf-8 in the update, so the body says whether this is the request we are waiting for. + const published = page.waitForRequest( + (request) => + request.url().includes('/collaboration/ydoc/v1/') && + request.method() === 'PATCH' && + (request.postDataBuffer()?.includes('Hello over http') ?? false), + { timeout: 45000 }, + ); + + await writeInEditor({ page, text: 'Hello over http' }); + + expect((await (await published).response())?.status()).toBe(200); + + // the round trip: the socket is still refused, so what comes back on reload came back + // over http + await page.reload(); + + await expect(page.getByText('Hello over http')).toBeVisible({ + timeout: 45000, + }); + }); + // TODO(yhub): Add test to check that no connected websocket users can collaborate test('checks disconnection and reconnection when changing tab visibility', async ({ diff --git a/src/frontend/apps/impress/package.json b/src/frontend/apps/impress/package.json index a1087be7b..0e8ec0145 100644 --- a/src/frontend/apps/impress/package.json +++ b/src/frontend/apps/impress/package.json @@ -54,6 +54,7 @@ "@tanstack/react-query": "5.102.2", "@tiptap/extension-find-and-replace": "3.30.6", "@tiptap/extensions": "*", + "@y/yhub-http-fallback": "0.1.1", "ai": "6.0.205", "canvg": "4.0.3", "clsx": "2.1.1", @@ -83,7 +84,7 @@ "uuid": "14.0.2", "y-prosemirror": "1.3.7", "y-protocols": "1.0.7", - "y-websocket": "3.0.0", + "y-websocket": "3.1.0", "yjs": "*", "zod": "4.4.3", "zustand": "5.0.15" diff --git a/src/frontend/apps/impress/src/core/config/hooks/useCollaborationUrl.tsx b/src/frontend/apps/impress/src/core/config/hooks/useCollaborationUrl.tsx index d87974c1c..a2d7f503b 100644 --- a/src/frontend/apps/impress/src/core/config/hooks/useCollaborationUrl.tsx +++ b/src/frontend/apps/impress/src/core/config/hooks/useCollaborationUrl.tsx @@ -16,3 +16,29 @@ export const useCollaborationUrl = (room?: string) => { : '') ); }; + +/** + * y/hub serves the same rooms over two transports, mounted side by side under one prefix: + * `{prefix}/ws/v1/{org}/{docid}` for the websocket and `{prefix}/ydoc/v1/{org}/{docid}` over + * plain http. The two providers spell a room differently — y-websocket takes a base url and + * appends the room name, the http provider takes the org and the docid apart — so this reads + * the org out of the websocket url and hands back what `HttpProvider` needs. + * + * Returns undefined for a url that is not shaped like one. An instance configured with + * something else then simply has no http fallback, rather than polling an address nobody + * serves. + */ +export const collaborationHttpTarget = (wsUrl: string) => { + const match = /^(ws|wss):\/\/(.*)\/ws\/v1\/([^/]+)\/?$/.exec(wsUrl); + + if (!match) { + return; + } + + const [, scheme, base, org] = match; + + return { + serverUrl: `${scheme === 'wss' ? 'https' : 'http'}://${base}`, + org, + }; +}; diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/stores/__tests__/useProviderStore.test.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/stores/__tests__/useProviderStore.test.tsx index f8b2e6980..153a1f026 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/stores/__tests__/useProviderStore.test.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/stores/__tests__/useProviderStore.test.tsx @@ -3,12 +3,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { useProviderStore } from '../useProviderStore'; /** - * A stand-in for y-websocket's provider, faithful on the two points these - * tests are about: the listeners it lets us register, and `shouldConnect`, - * which is what its retry loop reads before opening a socket again. + * A stand-in for y-websocket's provider, faithful on the points these tests are about: the + * listeners it lets us register, `shouldConnect`, which is what its retry loop reads before + * opening a socket again, and the close codes it treats as terminal — 4400-4499, where it + * stops reconnecting on its own and emits `closed`. */ class FakeProvider { public shouldConnect = true; + public synced = false; public connect = vi.fn(() => { this.shouldConnect = true; }); @@ -28,9 +30,53 @@ class FakeProvider { emit(event: string, ...args: unknown[]) { this.listeners[event]?.forEach((listener) => listener(...args)); } + + /** + * What y-websocket does on a close, in the same order: `connection-close` first, then — + * for a code its default `shouldReconnect` calls permanent — `shouldConnect = false` and + * `closed`. + */ + close(code: number | null) { + const event = code === null ? null : { code, reason: '' }; + this.emit('connection-close', event, this); + + if (event && event.code >= 4400 && event.code < 4500) { + this.shouldConnect = false; + this.emit('closed', event, this); + } + } +} + +/** + * A stand-in for `HttpProvider`. The store never drives its polling — that is + * `createWebsocketFallback`'s job — so what matters here is that the store stops it when it + * must, and that its `sync` reaches `isSynced`. + */ +class FakeHttpProvider { + public shouldConnect = false; + public synced = false; + public connect = vi.fn(() => { + this.shouldConnect = true; + }); + public disconnect = vi.fn(() => { + this.shouldConnect = false; + }); + public destroy = vi.fn(); + + private listeners: Record void)[]> = {}; + + on(event: string, listener: (...args: unknown[]) => void) { + (this.listeners[event] ??= []).push(listener); + } + + emit(event: string, ...args: unknown[]) { + this.listeners[event]?.forEach((listener) => listener(...args)); + } } let provider: FakeProvider; +let httpProvider: FakeHttpProvider; +let stopFallback: ReturnType; vi.mock('y-websocket', () => ({ // a function expression, not an arrow: the store builds it with `new` @@ -39,17 +85,47 @@ vi.mock('y-websocket', () => ({ }), })); -const closeWith = (code: number) => - provider.emit('connection-close', { code }, provider); +/** + * Stands in for `createWebsocketFallback`, emulating the one reaction the store has to order + * itself against: on `closed` the real helper starts the http provider, so the store's own + * `closed` handler has to run after it — lib0 emits to a snapshot of its listeners, so the + * `off()` inside one does not stop the ones registered after it. + */ +const createWebsocketFallback = vi.fn( + (primary: FakeProvider, secondary: FakeHttpProvider) => { + const onClosed = () => secondary.connect(); + primary.on('closed', onClosed); + return stopFallback; + }, +); + +vi.mock('@y/yhub-http-fallback', () => ({ + HttpProvider: vi.fn(function () { + return httpProvider; + }), + createWebsocketFallback: (primary: unknown, secondary: unknown) => + createWebsocketFallback( + primary as FakeProvider, + secondary as FakeHttpProvider, + ), +})); + +const closeWith = (code: number) => provider.close(code); describe('useProviderStore', () => { beforeEach(() => { vi.useFakeTimers(); provider = new FakeProvider(); + httpProvider = new FakeHttpProvider(); + stopFallback = vi.fn(); + createWebsocketFallback.mockClear(); // the store is a module-level singleton: put it back to its defaults, or // a test reads what the one before it left behind useProviderStore.getState().destroyProvider(); - useProviderStore.getState().createProvider('ws://localhost', 'doc-id'); + createWebsocketFallback.mockClear(); + useProviderStore + .getState() + .createProvider('ws://localhost/collaboration/ws/v1/docs', 'doc-id'); }); afterEach(() => { @@ -66,6 +142,8 @@ describe('useProviderStore', () => { // the document is refetched: the connection may have dropped because the // access to it changed expect(useProviderStore.getState().hasLostConnection).toBe(true); + // and the http fallback is left in place to take over + expect(stopFallback).not.toHaveBeenCalled(); }); it.each([ @@ -74,8 +152,13 @@ describe('useProviderStore', () => { ])('stops reconnecting on %s', (_label, code) => { closeWith(code); - // immediately, before the reconnection y-websocket has just scheduled - expect(provider.shouldConnect).toBe(false); + // nothing polls a document that has just been refused, and nothing revives + // the socket on a timer either + expect(stopFallback).toHaveBeenCalled(); + expect(httpProvider.disconnect).toHaveBeenCalled(); + // and it stays that way: the fallback also reacts to `closed`, so this only + // holds while the store has the last word on the event + expect(httpProvider.shouldConnect).toBe(false); vi.runAllTimers(); @@ -93,11 +176,12 @@ describe('useProviderStore', () => { expect(provider.shouldConnect).toBe(true); expect(useProviderStore.getState().isPermanentlyClosed).toBe(false); + expect(stopFallback).not.toHaveBeenCalled(); }); it('does not report a close it triggered itself as permanent', () => { // `destroy()` and `disconnect()` emit the event with no close event - provider.emit('connection-close', null, provider); + provider.close(null); vi.runAllTimers(); expect(useProviderStore.getState().isPermanentlyClosed).toBe(false); @@ -111,6 +195,8 @@ describe('useProviderStore', () => { expect(provider.connect).toHaveBeenCalled(); expect(useProviderStore.getState().isPermanentlyClosed).toBe(false); + // the fallback comes back with it - one install at createProvider, one here + expect(createWebsocketFallback).toHaveBeenCalledTimes(2); }); it('leaves a connection refused for good closed when the tab becomes active', () => { @@ -123,4 +209,36 @@ describe('useProviderStore', () => { expect(provider.connect).not.toHaveBeenCalled(); expect(useProviderStore.getState().isPermanentlyClosed).toBe(true); }); + + it('runs the http fallback on the same document and awareness', () => { + expect(createWebsocketFallback).toHaveBeenCalledTimes(1); + expect(createWebsocketFallback).toHaveBeenCalledWith( + provider, + httpProvider, + ); + expect(useProviderStore.getState().httpProvider).toBe(httpProvider); + }); + + it('reports the document as synced while it is the http fallback that syncs it', () => { + closeWith(1006); + expect(useProviderStore.getState().isSynced).toBe(false); + + httpProvider.synced = true; + httpProvider.emit('sync', true); + + // `useUpdateDoc` reads this to tell the backend that the collaboration + // server holds the content - true of either transport + expect(useProviderStore.getState().isSynced).toBe(true); + }); + + it('tears everything down with the document', () => { + useProviderStore.getState().destroyProvider(); + + expect(stopFallback).toHaveBeenCalled(); + expect(httpProvider.destroy).toHaveBeenCalled(); + expect(provider.destroy).toHaveBeenCalled(); + expect(provider.awareness.destroy).toHaveBeenCalled(); + expect(provider.doc.destroy).toHaveBeenCalled(); + expect(useProviderStore.getState().httpProvider).toBeUndefined(); + }); }); diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx index b5ee44bca..ba65927a4 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx @@ -1,7 +1,9 @@ +import { HttpProvider, createWebsocketFallback } from '@y/yhub-http-fallback'; import { WebsocketProvider } from 'y-websocket'; import * as Y from 'yjs'; import { create } from 'zustand'; +import { collaborationHttpTarget } from '@/core/config/hooks/useCollaborationUrl'; import { Base64 } from '@/docs/doc-management'; export interface UseCollaborationStore { @@ -15,6 +17,7 @@ export interface UseCollaborationStore { pauseForInactivity: () => void; resumeFromInactivity: () => void; provider: WebsocketProvider | undefined; + httpProvider: HttpProvider | undefined; isConnected: boolean; isReady: boolean; isSynced: boolean; @@ -27,6 +30,7 @@ export interface UseCollaborationStore { const defaultValues = { provider: undefined, + httpProvider: undefined, isConnected: false, isReady: false, isSynced: false, @@ -43,22 +47,36 @@ const defaultValues = { */ const RECONNECT_JITTER_MAX_MS = 3000; -/** - * Close codes 4400-4499 are the collaboration server refusing this connection - * rather than losing it: its access changed (4401) or the document was deleted - * (4404). It has answered, and reconnecting on a timer only asks the same - * question again — twice a minute, for as long as the tab stays open, for a - * document that may never come back. Everything else (a dropped socket, a - * server restart, an upgrade that failed) is transient and keeps its retry - * loop. - * - * Refused is not the same as gone: an access upgraded from reader to editor is - * a refusal too, and the connection has to be made again to carry the new - * rights. Asking the backend is what settles it, in `useCollaboration`. - */ -const isPermanentCloseCode = (code: number) => code >= 4400 && code <= 4499; - let lostConnectionTimeout: ReturnType | undefined; +/** + * Uninstalls the http fallback, or undefined while it is not installed. Held here rather than + * in the store: nothing renders from it, and it must survive `set(defaultValues)`. + */ +let stopFallback: (() => void) | undefined; + +/** + * Run the http provider only while the socket is not working, and stop it as soon as the socket + * is back. The helper follows `provider.shouldConnect`, so a connection closed from here — + * `pauseForInactivity` — stops both transports and revives neither until `connect()`. + */ +const installFallback = ( + provider: WebsocketProvider, + httpProvider: HttpProvider | undefined, +) => { + if (httpProvider && !stopFallback) { + stopFallback = createWebsocketFallback(provider, httpProvider); + } +}; + +/** + * Stop polling and stay stopped. The helper retries a provider that gave up every 30s, which is + * right for a network outage and wrong for a document that answered. + */ +const suspendFallback = (httpProvider: HttpProvider | undefined) => { + stopFallback?.(); + stopFallback = undefined; + httpProvider?.disconnect(); +}; export const useProviderStore = create((set, get) => ({ ...defaultValues, @@ -81,6 +99,39 @@ export const useProviderStore = create((set, get) => ({ resyncInterval: 20000, }); + /** + * A second transport onto the same document, for the networks that refuse websocket + * upgrades — corporate proxies, captive portals — where the socket above never opens and + * the editor would otherwise render the last snapshot and sync nothing. It polls y/hub's + * REST api on the same room, with the same session cookie and the same authorization, so + * the only thing that changes is latency. + * + * It shares the document, so nothing has to be flushed when the transport changes: what + * one provider retrieved is part of the document, and the other publishes it on its next + * sync. It shares the `Awareness` instance for the same reason — awareness state is keyed + * by `doc.clientID`, so two instances would advertise the same client id with independent + * clocks and fight over the local state. + */ + const target = collaborationHttpTarget(wsUrl); + const httpProvider = target + ? new HttpProvider( + doc, + target.serverUrl, + { org: target.org, docid: storeId }, + { + awareness: provider.awareness, + // createWebsocketFallback owns the connection state + connect: false, + // Docs users are served the garbage-collected document; a full-history request is + // refused by the collaboration server, and this defaults to `false` + gc: true, + // the session cookie is the credential here too, exactly as on the ws upgrade + fetch: (input, init) => + fetch(input, { ...init, credentials: 'include' }), + }, + ) + : undefined; + provider.on('status', ({ status }) => { // 'connecting' must be ignored: it fires on every backoff retry. // 'disconnected' is handled via 'connection-close' (it never fires @@ -92,58 +143,84 @@ export const useProviderStore = create((set, get) => ({ } }); - provider.on('sync', (isSynced: boolean) => { - set({ isSynced, isReady: true }); - }); + // Either transport being synced is what `useUpdateDoc` asks about: it decides whether the + // backend is told that the content is held by the collaboration server. + const syncState = () => + set({ + isSynced: provider.synced || (httpProvider?.synced ?? false), + isReady: true, + }); + + provider.on('sync', syncState); + httpProvider?.on('sync', syncState); // Fires on every close AND every failed connection attempt // (an auth failure surfaces as an upgrade-level 401, close code 1006). - // The event is null when the socket was closed from here. - provider.on('connection-close', (event) => { + provider.on('connection-close', () => { // Skip when the disconnect was triggered by inactivity: // reconnection only happens once the user becomes active again. if (get().isPausedForInactivity) { return; } - // The editor renders from the last snapshot while y-websocket retries + // The editor renders from the last snapshot, and the http fallback takes over, while + // y-websocket retries set({ isConnected: false, isReady: true }); clearTimeout(lostConnectionTimeout); // Jitter spreading: Math.random() generates a random delay to avoid // all clients invalidating their queries at the same time - const jitter = Math.random() * RECONNECT_JITTER_MAX_MS; - - if (event && isPermanentCloseCode(event.code)) { - /** - * Stop the retry loop. Assigning `shouldConnect` rather than calling - * `disconnect()`: this runs inside y-websocket's own close handling, - * and `disconnect()` closes the socket that is already closing, which - * re-enters this listener. The reconnection it has just scheduled reads - * the flag back when it fires, and gives up. - */ - provider.shouldConnect = false; - lostConnectionTimeout = setTimeout( - () => set({ isPermanentlyClosed: true }), - jitter, - ); - return; - } - lostConnectionTimeout = setTimeout( () => set({ hasLostConnection: true }), - jitter, + Math.random() * RECONNECT_JITTER_MAX_MS, + ); + }); + + // Installed before the `closed` listener below, and that order is the point: lib0 hands an + // event to a snapshot of its listeners, so unsubscribing from inside one does not stop the + // ones registered after it. The helper reacts to `closed` by starting the http provider; + // ours has to run last to be the one that has the final word. + installFallback(provider, httpProvider); + + /** + * The collaboration server refused this connection rather than losing it: its access + * changed (4401) or the document was deleted (4404). It has answered, and reconnecting on + * a timer only asks the same question again — for a document that may never come back, for + * as long as the tab stays open. y-websocket stops its retry loop on those codes by itself; + * this stops the http fallback with it, which would otherwise both poll a document we have + * just been refused and revive the socket every 30s. + * + * Refused is not the same as gone: an access upgraded from reader to editor is a refusal + * too, and the connection has to be made again to carry the new rights. Asking the backend + * is what settles it, in `useCollaboration`, which resumes through `reconnect` below. + */ + provider.on('closed', () => { + suspendFallback(httpProvider); + + // beats the hasLostConnection timer the close above has just armed + clearTimeout(lostConnectionTimeout); + lostConnectionTimeout = setTimeout( + () => set({ isPermanentlyClosed: true }), + Math.random() * RECONNECT_JITTER_MAX_MS, ); }); set({ provider, + httpProvider, }); return provider; }, destroyProvider: () => { - const provider = get().provider; + const { provider, httpProvider } = get(); + + stopFallback?.(); + stopFallback = undefined; + + // publishes a farewell awareness state, best effort, so the others see us leave + httpProvider?.destroy(); + if (provider) { /** * destroy() emits 'connection-close' synchronously before removing @@ -166,6 +243,7 @@ export const useProviderStore = create((set, get) => ({ } clearTimeout(lostConnectionTimeout); set({ isPausedForInactivity: true, hasLostConnection: false }); + // the fallback follows `shouldConnect`, so this stops the polling too get().provider?.disconnect(); }, resumeFromInactivity: () => { @@ -187,7 +265,15 @@ export const useProviderStore = create((set, get) => ({ * has confirmed the document is still there to open. */ reconnect: () => { + const { provider, httpProvider } = get(); + set({ isPermanentlyClosed: false }); - get().provider?.connect(); + + if (!provider) { + return; + } + + provider.connect(); + installFallback(provider, httpProvider); }, })); diff --git a/src/frontend/yarn.lock b/src/frontend/yarn.lock index 5fee8eb78..bab15a2cb 100644 --- a/src/frontend/yarn.lock +++ b/src/frontend/yarn.lock @@ -7109,6 +7109,14 @@ resolved "https://registry.yarnpkg.com/@xtuc/long/-/long-4.2.2.tgz#d291c6a4e97989b5c61d9acf396ae4fe133a718d" integrity sha512-NuHqBY1PB/D8xU6s/thBgOAiAP7HOYDQ32+BFZILJ8ivkUkAHQnWfn6WhL79Owj1qmUnoN/YPhktdIoucipkAQ== +"@y/yhub-http-fallback@0.1.1": + version "0.1.1" + resolved "https://registry.yarnpkg.com/@y/yhub-http-fallback/-/yhub-http-fallback-0.1.1.tgz#3095056de1ddd8ce9a460f3bc74e4ffe5e5cefe6" + integrity sha512-KcK2ZRSH22jxPXnFRKwgyvNnfzDYvHXsmX9Eoi0s74Zajyzv1vuviIEjZG0wxzkFLahLGUSFPsnU447OhR5moQ== + dependencies: + lib0 "^0.2.102" + y-protocols "^1.0.5" + "@zip.js/zip.js@^2.8.8": version "2.8.10" resolved "https://registry.yarnpkg.com/@zip.js/zip.js/-/zip.js-2.8.10.tgz#98a0cc7fdef9d6e227236271af412db02b18a5b2" @@ -13482,7 +13490,7 @@ react-intersection-observer@11.0.0: resolved "https://registry.yarnpkg.com/react-intersection-observer/-/react-intersection-observer-11.0.0.tgz#c388c46dd9c36386bd3a4e4fe1af80baf46c6074" integrity sha512-tF2PjXa//GcUmkCIcZR2qGsj6HwnuunFQdgeJ89BwFE6epB7E9yIzdr018H1khxG7DiMpIDvAne1GKQMFwn+fw== -"react-is-18@npm:react-is@^18.3.1", react-is@^18.3.1: +"react-is-18@npm:react-is@^18.3.1": version "18.3.1" resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.3.1.tgz#e83557dc12eae63a99e003a46388b1dcbb44db7e" integrity sha512-/LLMVyas0ljjAtoYiPqYiL8VWXzUUdThrmU5+n20DZv+a+ClRoevUzw5JxU+Ieh5/c87ytoTBV9G1FiKfNJdmg== @@ -13507,6 +13515,11 @@ react-is@^17.0.1: resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.2.tgz#e691d4a8e9c789365655539ab372762b0efb54f0" integrity sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w== +react-is@^18.3.1: + version "18.3.1" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.3.1.tgz#e83557dc12eae63a99e003a46388b1dcbb44db7e" + integrity sha512-/LLMVyas0ljjAtoYiPqYiL8VWXzUUdThrmU5+n20DZv+a+ClRoevUzw5JxU+Ieh5/c87ytoTBV9G1FiKfNJdmg== + react-lifecycles-compat@^3.0.0, react-lifecycles-compat@^3.0.4: version "3.0.4" resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362" @@ -14559,7 +14572,16 @@ string-length@^4.0.2: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0": + version "4.2.3" + resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" + integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== + dependencies: + emoji-regex "^8.0.0" + is-fullwidth-code-point "^3.0.0" + strip-ansi "^6.0.1" + +string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -14693,7 +14715,14 @@ stringify-object@^3.3.0: is-obj "^1.0.1" is-regexp "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1": + version "6.0.1" + resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" + integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== + dependencies: + ansi-regex "^5.0.1" + +strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -16269,10 +16298,10 @@ y-protocols@1.0.7, y-protocols@^1.0.5: dependencies: lib0 "^0.2.85" -y-websocket@3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/y-websocket/-/y-websocket-3.0.0.tgz#e86bdb29cc0a53cb8d6e33ec8d24614a723832af" - integrity sha512-mUHy7AzkOZ834T/7piqtlA8Yk6AchqKqcrCXjKW8J1w2lPtRDjz8W5/CvXz9higKAHgKRKqpI3T33YkRFLkPtg== +y-websocket@3.1.0: + version "3.1.0" + resolved "https://registry.yarnpkg.com/y-websocket/-/y-websocket-3.1.0.tgz#c6f6786ca1c0170b9a20f13c9cfc2cdb5c27d683" + integrity sha512-ZNzwH84Ysxv7zjpFNZHjTJvrBZgcAqMljTe+6zrWciAML9LQ18aVylyPNH9faxCXqEOV8I0JY4TGtrIHFX+Xwg== dependencies: lib0 "^0.2.102" y-protocols "^1.0.5" diff --git a/src/yhub-server/README.md b/src/yhub-server/README.md index 757937132..267617327 100644 --- a/src/yhub-server/README.md +++ b/src/yhub-server/README.md @@ -91,6 +91,26 @@ probes are not worth publishing either — kubelet calls them from inside — an the helm chart's ingress lists what it routes rather than what it hides, so they stay in-cluster on their own. +### Origins and cors + +`COLLABORATION_SERVER_ORIGIN` is the list of origins a browser may reach this +server from, and it is passed to yhub as its `cors` configuration: yhub applies +it to the websocket upgrade *and* to every REST route, refusing a cross-origin +request from anywhere else with a `403` before authentication runs. A request +carrying no `Origin` at all is same-origin or is not a browser, and is gated by +the session cookie alone — which is why `readAuthInfo` no longer checks the +origin itself: doing it twice would refuse exactly the requests the http +fallback makes, since a same-origin `fetch` GET sends no `Origin` header. + +`credentials: true` goes with it, so that browsers may send the session cookie +on a cross-origin request. That is what the frontend's http fallback +(`@y/yhub-http-fallback`, which polls `GET`/`PATCH /collaboration/ydoc/v1/…` +when a network refuses the websocket upgrade) needs, and it is also why the +list has to be concrete: browsers reject `Access-Control-Allow-Credentials` +together with a wildcard origin. Entries are bare origins — +`https://host[:port]`, no path, no trailing slash — or yhub refuses them at +startup. + ## Roles (`YHUB_ROLE`) yhub is two halves that share the two stores and nothing else — no in-process diff --git a/src/yhub-server/package-lock.json b/src/yhub-server/package-lock.json index 267f09f21..c9914474e 100644 --- a/src/yhub-server/package-lock.json +++ b/src/yhub-server/package-lock.json @@ -7,7 +7,7 @@ "name": "yhub-server", "dependencies": { "@aws-sdk/client-s3": "3.1110.0", - "@y/hub": "0.6.0", + "@y/hub": "0.7.0", "@y/y": "14.0.0-rc.24", "jose": "6.2.8" }, @@ -504,9 +504,9 @@ "license": "ISC" }, "node_modules/@y/hub": { - "version": "0.6.0", - "resolved": "https://registry.npmjs.org/@y/hub/-/hub-0.6.0.tgz", - "integrity": "sha512-EI22ikgpeh3FgWo045hSpHPt9SZU6q7KyXvvw2czuP9rvnsE8kus1traWLVH6n2Tmd8JCo/0n1bTzc13Ise1yg==", + "version": "0.7.0", + "resolved": "https://registry.npmjs.org/@y/hub/-/hub-0.7.0.tgz", + "integrity": "sha512-IPZKFEgJfuLBFDzJAa2ikPwbQHO7WTQgCSFsxqZa9HJdLn4Nbx8HJyLs9hZYtoquTG3MhMOASLifp60EYrghbw==", "license": "AGPL-3.0 OR PROPRIETARY", "dependencies": { "@y-crdt/yn": "^0.1.4", @@ -517,13 +517,13 @@ "pino": "^10.3.1", "postgres": "^3.4.3", "redis": "^5.10.0", - "uws": "github:uNetworking/uWebSockets.js#v20.57.0" + "uws": "github:uNetworking/uWebSockets.js#v20.69.0" }, "bin": { "yhub": "bin/yhub.js" }, "engines": { - "node": ">=22.0.0", + "node": "^22.9.0 || ^24.0.0 || ^26.0.0", "npm": ">=8.0.0" }, "funding": { diff --git a/src/yhub-server/package.json b/src/yhub-server/package.json index 368469697..2f9de5ce6 100644 --- a/src/yhub-server/package.json +++ b/src/yhub-server/package.json @@ -9,7 +9,7 @@ }, "dependencies": { "@aws-sdk/client-s3": "3.1110.0", - "@y/hub": "0.6.0", + "@y/hub": "0.7.0", "@y/y": "14.0.0-rc.24", "jose": "6.2.8" }, diff --git a/src/yhub-server/server.js b/src/yhub-server/server.js index 7b67dc1b7..228e9ec52 100644 --- a/src/yhub-server/server.js +++ b/src/yhub-server/server.js @@ -238,7 +238,9 @@ const backendFetch = async (path, { cookie, origin }) => { const res = await fetch(`${COLLABORATION_BACKEND_BASE_URL}${path}`, { headers: { cookie, - origin, + // a same-origin request carries no `Origin` — forwarded when there is one, omitted + // rather than sent empty, which is not a value the header is allowed to take + ...(origin ? { origin } : {}), 'X-Y-Provider-Key': Y_PROVIDER_API_KEY, }, }); @@ -339,7 +341,11 @@ const auth = createAuthPlugin({ } } if (gcOff) return null; // full-history connections: not for Docs users - if (!origin || !allowedOrigins.includes(origin)) return null; // was 4001 'Origin not allowed' + // No origin check here: `server.cors` below is the allowlist, and yhub applies it to the + // websocket upgrade and to every REST request before this runs. Checking it a second time + // would also refuse the http fallback's polls — a same-origin `fetch` GET carries no + // `Origin` header at all, so on a deployment where the page and /collaboration/ share a + // host every round would 401 while the PATCH beside it succeeded. if (!cookie) return null; // was 4001 'No cookies' try { const user = await backendFetch('/api/v1.0/users/me/', { @@ -953,7 +959,18 @@ const yhub = await createYHub({ // apiPrefix mounts every route — built-ins, our custom endpoints, and the // websocket (/collaboration/ws/v1/{org}/{docid}) — under /collaboration/. server: RUNS_SERVER - ? { port: PORT, auth, api, apiPrefix: API_PREFIX } + ? { + port: PORT, + auth, + api, + apiPrefix: API_PREFIX, + // What a browser may reach this server from, applied by yhub to the websocket upgrade + // and to every REST route — the only origin check there is, `readAuthInfo` no longer + // does its own. `credentials` is what lets the http fallback send the session cookie + // on a cross-origin `fetch`; it is also why the list has to be concrete, browsers + // refusing "*" together with Access-Control-Allow-Credentials. + cors: { origin: allowedOrigins, credentials: true }, + } : null, worker: RUNS_WORKER ? { taskConcurrency: TASK_CONCURRENCY, events: workerEvents }