Skip to content

Bound JSON nesting in Couchbase manifests - #3659

Open
wasphin wants to merge 1 commit into
apache:masterfrom
wasphin:fix-couchbase-manifest-json-depth
Open

wasphin wants to merge 1 commit into
apache:masterfrom
wasphin:fix-couchbase-manifest-json-depth

Conversation

@wasphin

@wasphin wasphin commented Oct 25, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

Problem Summary:

Couchbase manifest parsing currently accepts arbitrarily nested JSON and ignores bytes after an embedded NUL.

What is changed and the side effects?

Changed:

  • Validate manifest JSON with an iterative reader and a nesting limit of 100 before constructing the DOM.
  • Require the reader to consume the complete input and build the DOM iteratively.
  • Cover boundary depths, deeply nested arrays, quoted brackets, and embedded NULs.

Side effects:

  • Performance effects: Adds one linear validation pass over each manifest.
  • Breaking backward compatibility: Manifests deeper than 100 levels or containing embedded NULs are rejected. No API signature changes.

Check List:

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Source review found no blocking issues, and focused regression cases cover the intended behavior; tests were not executed during this review.

No open issues 🎉

What changed in this PR

This PR hardens Couchbase manifest parsing by limiting JSON nesting to 100 levels and rejecting input truncated by embedded NUL bytes.

Changes:

  • Adds iterative validation with depth and complete-input checks before constructing the DOM iteratively.
  • Adds regression coverage for boundary depths, deeply nested arrays, quoted brackets, and embedded NULs.
File Description
test/​brpc_couchbase_unittest.cpp Tests nesting limits and input-validation edge cases.
src/​brpc/​couchbase.cpp Enforces bounded nesting and complete-input validation before DOM construction.

Review effort: Balanced

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.

2 participants