Skip to content

Commit 39fbdda

Browse files
bernard-code-labclaude
authored andcommitted
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 8c7718e. 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>
1 parent b763af1 commit 39fbdda

1 file changed

Lines changed: 28 additions & 6 deletions

File tree

packages/platform-ws/adapters/ws-adapter.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,15 +177,37 @@ export class WsAdapter extends AbstractWsAdapter {
177177
handlersMap: Map<string, MessageMappingProperties>,
178178
transform: (data: any) => Observable<any>,
179179
): Observable<any> {
180+
let message: ReturnType<WsMessageParser>;
180181
try {
181-
const message = this.messageParser(buffer.data);
182-
if (!message) {
183-
return EMPTY;
182+
message = this.messageParser(buffer.data);
183+
} catch (err) {
184+
// A custom parser that throws is an application bug, and swallowing it
185+
// leaves no trace of it anywhere. The default parser, on the other hand,
186+
// throws on client-controlled input, so reporting that one would turn a
187+
// public socket into a log flood target. A custom parser that wants a
188+
// frame dropped silently can return nothing, which is handled below.
189+
if (this.messageParser !== defaultMessageParser) {
190+
this.logger.error(err);
184191
}
185-
const messageHandler = handlersMap.get(message.event)!;
186-
const { callback } = messageHandler;
187-
return transform(callback(message.data, message.event));
192+
return EMPTY;
193+
}
194+
if (!message) {
195+
return EMPTY;
196+
}
197+
198+
const messageHandler = handlersMap.get(message.event);
199+
if (!messageHandler) {
200+
// An unrecognised event is an expected condition on a public socket,
201+
// not an error. It used to reach the catch below as a TypeError.
202+
return EMPTY;
203+
}
204+
205+
try {
206+
return transform(messageHandler.callback(message.data, message.event));
188207
} catch {
208+
// Kept deliberately: a synchronous throw here would otherwise propagate
209+
// through the mergeMap in bindMessageHandlers and tear down the client's
210+
// source stream, silencing every subsequent message from that client.
189211
return EMPTY;
190212
}
191213
}

0 commit comments

Comments
 (0)