fix: avoid stack overflow in hasBinary() for objects with a toJSON() - #575
Open
junsj119 wants to merge 1 commit into
Open
fix: avoid stack overflow in hasBinary() for objects with a toJSON()#575junsj119 wants to merge 1 commit into
junsj119 wants to merge 1 commit into
Conversation
hasBinary() recursed into all own enumerable keys before checking for a custom toJSON() method. Objects that define toJSON() but also hold internal circular references — most notably Mongoose documents, whose `$__` cache points back to the document — overflowed the stack before the toJSON() short-circuit was ever reached. In sharded mode the resulting RangeError propagates through encode() -> doPublish() -> broadcast() and the event is dropped without any error surfaced (unless DEBUG is enabled). Check toJSON() before the key traversal, mirroring how the payload is actually serialized, so such objects are inspected through their toJSON() representation instead of their internal fields. Fixes socketio#572
junsj119
force-pushed
the
fix/hasbinary-tojson-circular
branch
from
August 7, 2026 04:28
41033dc to
59c7767
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
hasBinary()inlib/util.tsrecurses into all own enumerable keys before checking for a customtoJSON()method:Objects that define
toJSON()but also hold internal circular references overflow the stack before thetoJSON()short-circuit is ever reached. The clearest real-world case is a Mongoose document: its internal$__cache (InternalCache) points back to the document and into the schema/model graph, so thefor...intraversal never terminates.In sharded mode the resulting
RangeError: Maximum call stack size exceededpropagates throughencode()→doPublish()→broadcast(), and the event is silently dropped — no error is surfaced unlessDEBUG=socket.io-redisis enabled.Fixes #572.
Fix
Check
toJSON()before the key traversal. This mirrors how the payload is actually serialized (an object withtoJSON()is emitted through itstoJSON()output), so the function inspects the representation that will really be sent instead of walking the object's internal fields.Tests
Added
test/hasBinary.ts(no Redis required) covering:ArrayBuffer/TypedArray, nested, in arrays)toJSON()method no longer overflows the stacktoJSON()representationWithout the fix the last two cases throw
RangeError; with it all four pass. Wired intotest-runner.tsalongside the existing suites.