diff --git a/index.js b/index.js index 9361167..48c4c3b 100644 --- a/index.js +++ b/index.js @@ -110,6 +110,10 @@ function fastifyWebsocket (fastify, opts, next) { fastify.decorate('injectWS', injectWS) function onUpgrade (rawRequest, socket, head) { + // Node removes its socket error listener before emitting 'upgrade'. Keep + // the socket guarded while Fastify runs hooks and routes the request. + socket.on('error', onUpgradeSocketError) + // 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 rawRequest[kWsHead] = head @@ -124,6 +128,9 @@ function fastifyWebsocket (fastify, opts, next) { websocketListenServer.on('upgrade', onUpgrade) const handleUpgrade = (rawRequest, callback) => { + // `ws` installs its own socket error listener synchronously in + // handleUpgrade, so it is safe to hand error ownership over here. + rawRequest[kWs].removeListener('error', onUpgradeSocketError) wss.handleUpgrade(rawRequest, rawRequest[kWs], rawRequest[kWsHead], (socket) => { wss.emit('connection', socket, rawRequest) @@ -131,6 +138,10 @@ function fastifyWebsocket (fastify, opts, next) { }) } + function onUpgradeSocketError () { + this.destroy() + } + fastify.addHook('onRequest', (request, _reply, done) => { // this adds req.ws to the Request object if (request.raw[kWs]) { request.ws = true diff --git a/test/base.test.js b/test/base.test.js index ee38023..f1e0759 100644 --- a/test/base.test.js +++ b/test/base.test.js @@ -1,6 +1,7 @@ 'use strict' const http = require('node:http') +const net = require('node:net') const split = require('split2') const { test } = require('node:test') const Fastify = require('fastify') @@ -223,6 +224,45 @@ test('Should run custom errorHandler when the raw socket emits an error', async await p }) +test('Should handle raw socket errors while upgrade hooks are running', async (t) => { + const fastify = Fastify() + t.after(() => fastify.close()) + + await fastify.register(fastifyWebsocket) + + let enterHook + const hookEntered = new Promise((resolve) => { + enterHook = resolve + }) + + fastify.addHook('onRequest', async () => { + enterHook() + await new Promise((resolve) => setTimeout(resolve, 100)) + }) + + fastify.get('/', { websocket: true }, () => {}) + + await fastify.listen({ port: 0 }) + + const client = net.connect(fastify.server.address().port, '127.0.0.1', () => { + client.write( + 'GET / HTTP/1.1\r\n' + + 'Host: localhost\r\n' + + 'Connection: Upgrade\r\n' + + 'Upgrade: websocket\r\n' + + 'Sec-WebSocket-Version: 13\r\n' + + 'Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\r\n' + ) + }) + client.on('error', () => {}) + + await hookEntered + client.resetAndDestroy() + await new Promise((resolve) => setTimeout(resolve, 200)) + + t.assert.strictEqual(fastify.server.listening, true) +}) + test('Should be able to pass custom options to ws', async (t) => { t.plan(2)