Skip to content

Add recursion depth limit to Node.js protocol skip()#3389

Open
Jens-G wants to merge 1 commit intoapache:masterfrom
Jens-G:p1-node-limit-skip
Open

Add recursion depth limit to Node.js protocol skip()#3389
Jens-G wants to merge 1 commit intoapache:masterfrom
Jens-G:p1-node-limit-skip

Conversation

@Jens-G
Copy link
Copy Markdown
Member

@Jens-G Jens-G commented Apr 11, 2026

Client: nodejs

Add a depth parameter to skip() in TBinaryProtocol, TCompactProtocol, and TJSONProtocol to prevent stack exhaustion from deeply nested structures. Limit set to 64, matching the C++ and Java implementations.

Client: nodejs

Add a depth parameter to skip() in TBinaryProtocol, TCompactProtocol,
and TJSONProtocol to prevent stack exhaustion from deeply nested
structures. Limit set to 64, matching the C++ and Java implementations.

(01)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mergeable mergeable bot added the nodejs label Apr 11, 2026
Copy link
Copy Markdown
Member

@jimexist jimexist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if 64 is meant to be made configurable?

@Jens-G
Copy link
Copy Markdown
Member Author

Jens-G commented Apr 12, 2026

The limit of 64 matches TConfiguration.DEFAULT_RECURSION_DEPTH which is the standard default across languages. Node.js currently has no TConfiguration infrastructure, so making it configurable would require adding that first — feels offtopic here. But yes, we should keep that in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants