Closed dbulic-margins closed 1 year ago
Should we emit an exception
event when the URL is incorrect? Something along the lines of
protected ensureHttpServerExists(
port: number,
httpServer = http.createServer(),
) {
if (this.httpServersRegistry.has(port)) {
return;
}
this.httpServersRegistry.set(port, httpServer);
httpServer.on('upgrade', (request, socket, head) => {
try {
const baseUrl = 'ws://' + request.headers.host + '/';
const pathname = new URL(request.url, baseUrl).pathname;
const wsServersCollection = this.wsServersRegistry.get(port);
let isRequestDelegated = false;
for (const wsServer of wsServersCollection) {
if (pathname === wsServer.path) {
wsServer.handleUpgrade(request, socket, head, (ws: unknown) => {
wsServer.emit('connection', ws, request);
});
isRequestDelegated = true;
break;
}
}
if (!isRequestDelegated) {
socket.destroy();
}
} catch (err) {
socket.write(err.message);
socket.end();
}
});
return httpServer;
}
We should catch that error at the very least to avoid breaking the app. Would you like to create a PR for this @dbulic-margins?
Let's track this here https://github.com/nestjs/nest/pull/12385
Is there an existing issue for this?
Current behavior
I have reconnecting WebSocket on the frontend that is trying to connect to the WebSocket gateway on ws://localhost:3000/ . Mistakenly the URL requested is
ws://localhost:3000//
which is invalid URL (because of two slashes at the end) and platform-ws throws TypeError that crashes entire backend app:Minimum reproduction code
https://github.com/nestjs/nest/blob/5bba7e9d264319490f142ca5e8099c559fa7e7e3/packages/platform-ws/adapters/ws-adapter.ts#L182
Steps to reproduce
No response
Expected behavior
URL parsing should have some fallback action, or error handling. const pathname = new URL(request.url, baseUrl).pathname; https://github.com/nestjs/nest/blob/5bba7e9d264319490f142ca5e8099c559fa7e7e3/packages/platform-ws/adapters/ws-adapter.ts#L182
Package
Other package
No response
NestJS version
9.5.0
Packages versions
Node.js version
18.17.1
In which operating systems have you tested?
Other
No response