Skip to content

fix: enforce maximumDepth for nested objects with array replacers#64

Open
Chamath-Adithya wants to merge 1 commit into
BridgeAR:mainfrom
Chamath-Adithya:fix-array-replacer-depth-limit
Open

fix: enforce maximumDepth for nested objects with array replacers#64
Chamath-Adithya wants to merge 1 commit into
BridgeAR:mainfrom
Chamath-Adithya:fix-array-replacer-depth-limit

Conversation

@Chamath-Adithya
Copy link
Copy Markdown

Pull Request: Enforce maximumDepth for nested objects in array replacers

Description

This pull request addresses a bug in the array-replacer-based stringification path (stringifyArrayReplacer). Currently, the maximumDepth option is checked and enforced correctly when serializing arrays, but the check is completely bypassed when traversing nested objects. This allows nested objects to serialize beyond the configured maximumDepth boundary and potentially trigger stack overflows on heavily nested structures.

With this fix, stringifyArrayReplacer correctly enforces the maximumDepth constraint on nested objects, ensuring consistent behavior with other stringifier paths (stringifySimple, stringifyFnReplacer, and stringifyIndent).


Solution

Inside stringifyArrayReplacer (in index.js), added the maximumDepth comparison block right before pushing an object onto the traversal stack:

if (maximumDepth < stack.length + 1) {
  return '"[Object]"'
}

This perfectly mirrors the boundary check implemented in the other serialization paths.


Verification & Testing

1. New/Updated Tests

  • Adjusted the assertion of res2 (which tests maximumDepth: 2 in combination with an array replacer) in test.js to correctly expect '{"a":{"a":"[Array]","b":"[Object]"}}' instead of {}.
  • Added a new targeted unit test using maximumDepth: 1 in combination with an array replacer, verifying that it correctly truncates to '{"a":"[Object]"}'.

2. Standard Compliance & Style

Ran the tap tests and standard linter successfully:

npm test
npm run lint

All 74,937 assertions passed with 100% code coverage.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant