Skip to content

Core: limit depth of received JSON to 16 - #6378

Merged
black-sliver merged 3 commits into
ArchipelagoMW:mainfrom
black-sliver:feat/multiserver-safer-json
Aug 10, 2026
Merged

Core: limit depth of received JSON to 16#6378
black-sliver merged 3 commits into
ArchipelagoMW:mainfrom
black-sliver:feat/multiserver-safer-json

Conversation

@black-sliver

Copy link
Copy Markdown
Member

What is this fixing or adding?

Limits the maximum depth of a received JSON document to 16.

This applies to both MultiServer and CommonClient.

This means DataStorage default is limited to 14 levels, DataStorage op arg is limited to 13 levels.

How was this tested?

Unit tests.

This applies to both MultiServer and CommonClient.

This means DataStorage default is limited to 14 levels,
DataStorage op arg is limited to 13 levels.
@github-actions github-actions Bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Aug 5, 2026
@GinjaNinja32

Copy link
Copy Markdown

this doesn't handle backslash-escaped quotes, and mismatched backslash-escaped quotes go particularly badly:

{"command": "escaped unclosed \"quote"} => "malformed" despite being valid JSON
{..., {"at_max_depth": "a string with \"quoted [things]\""}, ...} => "too complex" despite not actually being more complex than a version saying "... \"quoted things\""
{..., {"at_max_depth": "\"]]]]\"", "a_complex_object": ..., "whoops": "\"[[[[\""}, ...} => passes despite being more complex than permitted

@black-sliver

Copy link
Copy Markdown
Member Author

Oh, true. Why did i forget about this? :/ I blame the summer.

@black-sliver

Copy link
Copy Markdown
Member Author

Some notes on the cython implementation:

  • While the python implementation is actually pretty slow, the cython one is pretty fast. For a "real" WebHost, I'd strongly recommend to make sure _speedups are built.
  • Uses cpdef rather than def to let the compiler generate an optimal cdef that then gets called from or inlined into the public def. cpdef can be faster than def (and for my test setup it was).
  • Uses Py_ssize_t to make sure the integer math can't overflow as long as the limit argument is less than SSIZE_MAX and can't underflow at all because 1 - SSIZE_MAX > SSIZE_MIN. CPython's str length is a Py_ssize_t.

@black-sliver
black-sliver merged commit fe5b49e into ArchipelagoMW:main Aug 10, 2026
18 checks passed
@github-actions github-actions Bot removed the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

affects: core Issues/PRs that touch core and may need additional validation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants