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 }