graasp/graasp-plugin-websockets

Fix a bug where connections to the ws endpoint without a valid session would crash the server

Closed this issue · 1 comments

When running the graasp server with the graasp-websockets plugin, accessing the websocket endpoint crashes the server if accessed using a websocket connection without a valid sesion.

If accessed through HTTP, no error is raised and the server doesn't crash.
If accessed through WS, the whole process crashes with error:

graasp-websockets: an error occured: Error: no valid session
[Node] 	Destroying connection [object Object]...
[Node] events.js:353
[Node]       throw er; // Unhandled 'error' event
[Node]       ^
[Node] 
[Node] Error: no valid session
[Node]     at Object.<anonymous> (/home/momochan/dev/epfl/graasp/dist/plugins/auth/auth.js:45:23)
[Node]     at Generator.next (<anonymous>)
[Node]     at /home/momochan/dev/epfl/graasp/dist/plugins/auth/auth.js:8:71
[Node]     at new Promise (<anonymous>)
[Node]     at __awaiter (/home/momochan/dev/epfl/graasp/dist/plugins/auth/auth.js:4:12)
[Node]     at Object.validateSession (/home/momochan/dev/epfl/graasp/dist/plugins/auth/auth.js:40:16)
[Node]     at hookIterator (/home/momochan/dev/epfl/graasp/node_modules/fastify/lib/hooks.js:237:10)
[Node]     at next (/home/momochan/dev/epfl/graasp/node_modules/fastify/lib/hooks.js:164:16)
[Node]     at hookRunner (/home/momochan/dev/epfl/graasp/node_modules/fastify/lib/hooks.js:187:3)
[Node]     at preValidationCallback (/home/momochan/dev/epfl/graasp/node_modules/fastify/lib/handleRequest.js:99:5)
[Node]     at handler (/home/momochan/dev/epfl/graasp/node_modules/fastify/lib/handleRequest.js:70:7)
[Node]     at handleRequest (/home/momochan/dev/epfl/graasp/node_modules/fastify/lib/handleRequest.js:18:5)
[Node]     at runPreParsing (/home/momochan/dev/epfl/graasp/node_modules/fastify/lib/route.js:421:5)
[Node]     at next (/home/momochan/dev/epfl/graasp/node_modules/fastify/lib/hooks.js:158:7)
[Node]     at internalNext (/home/momochan/dev/epfl/graasp/node_modules/helmet/dist/index.js:86:6)
[Node]     at xXssProtectionMiddleware (/home/momochan/dev/epfl/graasp/node_modules/helmet/dist/middlewares/x-xss-protection/index.js:6:3)
[Node] Emitted 'error' event on Duplex instance at:
[Node]     at Duplex.duplexOnError (/home/momochan/dev/epfl/graasp/node_modules/ws/lib/stream.js:37:10)
[Node]     at Duplex.emit (events.js:376:20)
[Node]     at Duplex.emit (domain.js:470:12)
[Node]     at emitErrorNT (internal/streams/destroy.js:106:8)
[Node]     at emitErrorCloseNT (internal/streams/destroy.js:74:3)
[Node]     at processTicksAndRejections (internal/process/task_queues.js:82:21)

Thus the error is probably in the errorHandler passed here: https://github.com/graasp/graasp-websockets/blob/4a55f781d29f49c0ff5766052d7f27f8fcc53f81/src/service-api.ts#L53-L58

Indeed, the bug was caused by conn.destroy(error).

The Stream.Readable API (https://nodejs.org/api/stream.html#stream_readable_destroy_error) raises the passed error optionally. We don't need the error to be processed by the stream itself, so we should just destroy the connection without passing the error.

Hence fix is 16670d7