From 42565db3a8775e504b2bed76411e044f4aa6967a Mon Sep 17 00:00:00 2001 From: MarkXian Date: Thu, 16 Jul 2026 16:40:46 +0800 Subject: [PATCH] fix: scope websocket upgrades to the proxy prefix The plugin registers a single server-wide `upgrade` listener that dispatches every WebSocket upgrade through `fastify.routing`, regardless of the prefix the proxy is mounted on. When the proxy shares the server with another `upgrade` handler (for example `@fastify/websocket`), it consumes and answers upgrades it does not own, breaking those unrelated WebSocket endpoints. Only handle upgrades whose path falls within a registered proxy prefix when the proxy coexists with other `upgrade` listeners. When the proxy is the only listener, behaviour is unchanged: every upgrade is routed as before, so out-of-prefix requests are still rejected normally. Closes #414 --- index.js | 33 +++++++++++- test/ws-upgrade-prefix.js | 105 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 test/ws-upgrade-prefix.js diff --git a/index.js b/index.js index c5f0947..7c84a04 100644 --- a/index.js +++ b/index.js @@ -14,6 +14,7 @@ const urlPattern = /^https?:\/\// const kWs = Symbol('ws') const kWsHead = Symbol('wsHead') const kWsUpgradeListener = Symbol('wsUpgradeListener') +const kWsPrefixes = Symbol('wsPrefixes') function liftErrorCode (code) { /* c8 ignore start */ @@ -361,6 +362,19 @@ function proxyWebSocketsWithReconnection (logger, source, target, options, hooks }) } +function isUpgradeWithinPrefixes (rawRequest, prefixes) { + const pathname = rawRequest.url.split('?', 1)[0] + for (const prefix of prefixes) { + // Normalise the prefix to its path without a trailing slash. A proxy + // mounted at the root ('' or '/') owns every upgrade. + const base = prefix.endsWith('/') ? prefix.slice(0, -1) : prefix + if (base === '' || pathname === base || pathname.startsWith(`${base}/`)) { + return true + } + } + return false +} + function handleUpgrade (fastify, rawRequest, socket, head) { // Save a reference to the socket and then dispatch the request through the normal fastify router so that it will invoke hooks and then eventually a route handler that might upgrade the socket. rawRequest[kWs] = socket @@ -394,10 +408,25 @@ class WebSocketProxy { }) if (!fastify.server[kWsUpgradeListener]) { - fastify.server[kWsUpgradeListener] = (rawRequest, socket, head) => - handleUpgrade(fastify, rawRequest, socket, head) + // A single 'upgrade' listener is shared by every proxy registered on + // this server. When the proxy coexists with other 'upgrade' listeners + // (e.g. @fastify/websocket), it must only handle upgrades that target + // one of the registered proxy prefixes, otherwise it would hijack + // WebSocket endpoints it does not own. When the proxy is the only + // 'upgrade' listener, every upgrade is dispatched as before so that + // out-of-prefix requests are still routed (and rejected) normally. + const prefixes = fastify.server[kWsPrefixes] = [] + fastify.server[kWsUpgradeListener] = (rawRequest, socket, head) => { + if ( + fastify.server.listenerCount('upgrade') === 1 || + isUpgradeWithinPrefixes(rawRequest, prefixes) + ) { + handleUpgrade(fastify, rawRequest, socket, head) + } + } fastify.server.on('upgrade', fastify.server[kWsUpgradeListener]) } + fastify.server[kWsPrefixes].push(fastify.prefix) this.handleUpgrade = (request, dest, cb) => { wss.handleUpgrade(request.raw, request.raw[kWs], request.raw[kWsHead], (socket) => { diff --git a/test/ws-upgrade-prefix.js b/test/ws-upgrade-prefix.js new file mode 100644 index 0000000..2bd6cc9 --- /dev/null +++ b/test/ws-upgrade-prefix.js @@ -0,0 +1,105 @@ +'use strict' + +const { test } = require('node:test') +const assert = require('node:assert') +const Fastify = require('fastify') +const proxy = require('../') +const WebSocket = require('ws') +const { createServer } = require('node:http') +const { promisify } = require('node:util') +const { once } = require('node:events') + +// Spin up an upstream echo server that prefixes every message it receives, so +// the proxied payload can be told apart from a locally handled one. +async function createEchoUpstream (t) { + const origin = createServer() + const wss = new WebSocket.Server({ server: origin }) + wss.on('connection', (ws) => { + ws.on('message', (message) => ws.send(`proxied:${message}`)) + }) + t.after(() => { wss.close() }) + t.after(() => { origin.close() }) + await promisify(origin.listen.bind(origin))({ port: 0, host: '127.0.0.1' }) + return `ws://127.0.0.1:${origin.address().port}` +} + +// A second, independent WebSocket endpoint mounted directly on the raw server. +// Its 'upgrade' listener is registered after the proxy's, so in the buggy +// behaviour the proxy hijacks (and 404s) the socket before this one can run. +function mountStandalone (t, server, ownedPath) { + const wss = new WebSocket.Server({ noServer: true }) + wss.on('connection', (ws) => { + ws.on('message', (message) => ws.send(`standalone:${message}`)) + }) + t.after(() => { wss.close() }) + server.server.on('upgrade', (rawRequest, socket, head) => { + if (rawRequest.url.split('?', 1)[0] === ownedPath) { + wss.handleUpgrade(rawRequest, socket, head, (ws) => { + wss.emit('connection', ws, rawRequest) + }) + } + }) +} + +async function connect (port, path) { + const ws = new WebSocket(`ws://127.0.0.1:${port}${path}`) + await once(ws, 'open') + ws.send('hello') + const [reply] = await once(ws, 'message') + ws.close() + await once(ws, 'close') + return reply.toString() +} + +test('does not hijack websocket upgrades outside a prefixed proxy', async (t) => { + const upstream = await createEchoUpstream(t) + + const server = Fastify() + server.register(proxy, { prefix: '/proxied', upstream, websocket: true }) + await server.listen({ port: 0, host: '127.0.0.1' }) + t.after(() => { server.close() }) + const port = server.server.address().port + + mountStandalone(t, server, '/standalone') + + // Out of prefix: must reach the standalone handler, untouched by the proxy. + assert.strictEqual(await connect(port, '/standalone'), 'standalone:hello') + // In prefix (exact match): still proxied. + assert.strictEqual(await connect(port, '/proxied'), 'proxied:hello') + // In prefix (nested path): still proxied. + assert.strictEqual(await connect(port, '/proxied/nested'), 'proxied:hello') +}) + +test('a prefix with a trailing slash is scoped correctly', async (t) => { + const upstream = await createEchoUpstream(t) + + const server = Fastify() + server.register(proxy, { prefix: '/pub/', upstream, websocket: true }) + await server.listen({ port: 0, host: '127.0.0.1' }) + t.after(() => { server.close() }) + const port = server.server.address().port + + mountStandalone(t, server, '/standalone') + + // Out of prefix: handled by the standalone endpoint. + assert.strictEqual(await connect(port, '/standalone'), 'standalone:hello') + // In prefix: proxied to the upstream. + assert.strictEqual(await connect(port, '/pub/nested'), 'proxied:hello') +}) + +test('a root proxy still owns upgrades when coexisting with another listener', async (t) => { + const upstream = await createEchoUpstream(t) + + const server = Fastify() + server.register(proxy, { upstream, websocket: true }) + await server.listen({ port: 0, host: '127.0.0.1' }) + t.after(() => { server.close() }) + const port = server.server.address().port + + // Present only so the proxy is not the sole 'upgrade' listener; it claims a + // path the root proxy is never asked about. + mountStandalone(t, server, '/reserved') + + // The root proxy owns every path, so an arbitrary path is still proxied. + assert.strictEqual(await connect(port, '/anything'), 'proxied:hello') +})