Skip to content

Limit nested container depth in Redis reply and AMF parsers#3344

Open
wwbmmm wants to merge 2 commits into
apache:masterfrom
wwbmmm:limit-depth-in-redis-and-amf-parsers
Open

Limit nested container depth in Redis reply and AMF parsers#3344
wwbmmm wants to merge 2 commits into
apache:masterfrom
wwbmmm:limit-depth-in-redis-and-amf-parsers

Conversation

@wwbmmm

@wwbmmm wwbmmm commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: null

Problem Summary:

Redis replies and AMF payloads can contain nested container values. The parsers previously bounded per-container allocation sizes, but did not bound nesting depth. Very deep inputs could therefore consume excessive call stack while parsing.

What is changed and the side effects?

Changed:

  • Add redis_max_reply_depth to limit nested Redis array replies.
  • Add amf_max_depth to limit nested AMF objects and arrays.
  • Add regression tests for over-limit nested Redis replies and AMF strict arrays.

Side effects:

  • Performance effects: negligible; parsing adds one integer depth check per nested container.
  • Breaking backward compatibility: inputs with Redis reply or AMF nesting deeper than the configured limit are rejected.

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.

Pull request overview

This PR adds configurable maximum nesting-depth limits to the Redis reply and AMF parsers to prevent excessively deep inputs from consuming too much call stack during parsing.

Changes:

  • Add redis_max_reply_depth and enforce it during recursive Redis array reply parsing.
  • Add amf_max_depth and enforce it during recursive AMF object/array parsing.
  • Add regression tests for over-limit nested Redis arrays and AMF strict arrays.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/brpc_rtmp_unittest.cpp Adds a scoped override for amf_max_depth and a regression test for deeply nested AMF strict arrays.
test/brpc_redis_unittest.cpp Adds a scoped override for redis_max_reply_depth and a regression test for deeply nested Redis array replies.
src/brpc/redis_reply.h Adds a private depth-tracking overload for RedisReply::ConsumePartialIOBuf.
src/brpc/redis_reply.cpp Defines redis_max_reply_depth and enforces the limit during recursive array parsing.
src/brpc/amf.cpp Defines amf_max_depth and enforces it in recursive AMF object/array parsing helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/brpc_rtmp_unittest.cpp
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