Repository navigation
fix(key): Answer HTTP 400 for JSON nested over 64 levels - #61
Merged
Merged
Conversation
dcramer
marked this pull request as ready for review
November 5, 2026 19:34
Member
|
Max depth can be far lower than this |
The key walks a parsed JSON body with recursion (`stableStringify` and the thinking-block walk of `request-key.ts`). A body nested some thousands of levels deep, such as 100,000 `[` and `]`, filled the call stack. The `RangeError` had no status, so the client got HTTP 502 and Sentry got an error event, for a run with or without the token. `keyRequest()` now turns that `RangeError` into an error with HTTP status 400. The key worker passes the status on, so both paths answer 400 and send nothing to Sentry. The run goes on. No key changes. The walk is the same code, so a body that got a key on main gets the same key. A body that filled the stack on main got no key, and it gets none now. The depth where V8 fills the stack depends on JIT tiers, on main too (4,500 to 8,000 levels of `[` in one process). The pinned key tests pass without a change, and a randomized compare of 30,000 bodies against main gives the same keys. Co-Authored-By: David Cramer <23610+dcramer@users.noreply.github.com>
Catching RangeError also caught a regex stack overflow from a client value pattern, with a wrong message, and could hide a real recursion bug. Check the depth of the parsed body in a loop before the key walks it, and fail over 1,000 levels with HTTP 400. On Node 24 the key overflowed from about 2,500 to 3,000 levels, so bodies between 1,001 and that edge, which got a key before, now fail. Keys of all other bodies do not change. Co-Authored-By: David Cramer <23610+dcramer@users.noreply.github.com>
Model request bodies nest far less than this. Use the shared httpError.
sentry-junior
Bot
force-pushed
the
fix/deep-json-key
branch
from
November 5, 2026 19:39
9d805db to
ce669c8
Compare
sentry-junior
Bot
changed the title
3 tasks done
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.
A JSON body nested a few thousand levels deep overflowed the stack in the key code, so the client got 502 and the error went to Sentry.
Now
request-key.tswalks the body without recursion before it keys it. A body nested over 64 levels (MAX_JSON_DEPTH) gets 400 "the JSON body is nested over 64 levels deep". The run keeps going, and nothing goes to Sentry. This replaces an earlier version that caught theRangeError, which also caught regex overflows with the wrong message.Behavior change: on main, keys overflowed at about 2,500–3,000 levels on Node 24, and the exact point varied between runs. Bodies from 65 levels to that point used to get a key and now get 400. Model request bodies nest far less than 64 levels.
Keys: unchanged for every body under the limit. The pinned key tests pass, and a 30,000-body comparison against main gave identical keys.
Cost: the extra walk takes about 8.6 ms on a 16 MiB body, against about 204 ms for the key.
Tests: 65 and 100,000 levels get 400 on the replay and writer paths, with no live call and no
Request failedlog. 64 levels gets 412 on replay and 200 on the writer. The test fails on main withexpected 502 to be 400.pnpm checkpasses.Uses the shared
httpErrorfrom #62.via @dcramer.
--
View Session [Sentry]