mirror of
https://github.com/nestjs/nest.git
synced 2026-10-29 02:06:11 +08:00
fix(ws): stop discarding parser and handler lookup failures silently
The message handler binding wrapped its whole body in a single try with an
empty catch, so three unrelated failures ended up indistinguishable from a
dropped packet: a custom message parser that throws, a malformed frame, and an
event with no registered handler.
The handler lookup used a non-null assertion followed by a destructure, so an
unrecognised event reached that catch as a TypeError. It is now an explicit
guard that keeps the single map lookup introduced in 8c7718e38.
A custom parser that throws is now reported, since nothing else in the process
would reveal it. The default parser stays silent because it throws on
client-controlled input, and reporting that would turn a public socket into a
log flood target.
The catch around the handler invocation is kept on purpose: without it a
synchronous throw propagates through the mergeMap that binds the client and
tears down that client's source stream, silencing every later message from it.
Closes #17624
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
b763af1736
commit
39fbddae51
1 file changed
+28
-6
@@ -177,15 +177,37 @@ export class WsAdapter extends AbstractWsAdapter {
|
||||
handlersMap: Map<string, MessageMappingProperties>,
|
||||
transform: (data: any) => Observable<any>,
|
||||
): Observable<any> {
|
||||
let message: ReturnType<WsMessageParser>;
|
||||
try {
|
||||
const message = this.messageParser(buffer.data);
|
||||
if (!message) {
|
||||
return EMPTY;
|
||||
message = this.messageParser(buffer.data);
|
||||
} catch (err) {
|
||||
// A custom parser that throws is an application bug, and swallowing it
|
||||
// leaves no trace of it anywhere. The default parser, on the other hand,
|
||||
// throws on client-controlled input, so reporting that one would turn a
|
||||
// public socket into a log flood target. A custom parser that wants a
|
||||
// frame dropped silently can return nothing, which is handled below.
|
||||
if (this.messageParser !== defaultMessageParser) {
|
||||
this.logger.error(err);
|
||||
}
|
||||
const messageHandler = handlersMap.get(message.event)!;
|
||||
const { callback } = messageHandler;
|
||||
return transform(callback(message.data, message.event));
|
||||
return EMPTY;
|
||||
}
|
||||
if (!message) {
|
||||
return EMPTY;
|
||||
}
|
||||
|
||||
const messageHandler = handlersMap.get(message.event);
|
||||
if (!messageHandler) {
|
||||
// An unrecognised event is an expected condition on a public socket,
|
||||
// not an error. It used to reach the catch below as a TypeError.
|
||||
return EMPTY;
|
||||
}
|
||||
|
||||
try {
|
||||
return transform(messageHandler.callback(message.data, message.event));
|
||||
} catch {
|
||||
// Kept deliberately: a synchronous throw here would otherwise propagate
|
||||
// through the mergeMap in bindMessageHandlers and tear down the client's
|
||||
// source stream, silencing every subsequent message from that client.
|
||||
return EMPTY;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user