Limit multibulk args count and fix integer type truncations

[wufengwind]

Add PROTO_MAX_MULTIBULK_LEN (1M) to prevent OOM from large argv allocations. Use ztrymalloc/ztryrealloc so allocation failure returns a protocol error instead of crashing.

Replace fragile unsigned subtraction overflow cast with explicit size_t checks in parseMultibulk.

Add SIZE_MAX guard in rdbGenericLoadStringObject for 32-bit platforms.

Fix sdsIncrLen type truncation where ssize_t was cast to unsigned int, losing upper bits on 64-bit.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds PROTO_MAX_MULTIBULK_LEN and a new READ_FLAGS_ERROR_TOO_MANY_ARGS flag, wired into multibulk parsing to cap argument count, return protocol errors, and use try-alloc on argv allocation. Also adds a SIZE_MAX overflow guard in RDB string loading and standardizes sdsIncrLen assertions with a new sdsIncrLenAbs helper.

Changes

Too-many-args multibulk protocol enforcement

Layer / File(s)	Summary
New constants, error flag, and error-reply wiring src/server.h, src/networking.c	PROTO_MAX_MULTIBULK_LEN (1024*1024) and READ_FLAGS_ERROR_TOO_MANY_ARGS (1<<23) are defined; handleParseError() gains a branch emitting "Protocol error: too many arguments" and isParsingError() includes the new flag.
Multibulk parsing: bounds checks, length cap, and try-alloc src/networking.c	Header and bulk-element \r\n completeness checks rewritten to use remaining/line_len; ll > PROTO_MAX_MULTIBULK_LEN now returns READ_FLAGS_ERROR_TOO_MANY_ARGS; zmalloc/zrealloc replaced with ztrymalloc/ztryrealloc, returning READ_FLAGS_ERROR_TOO_MANY_ARGS on failure.
Protocol test: too many multibulk arguments tests/unit/protocol.tcl	New test sends *1048577\r\n and asserts the server replies with an error containing "too many arguments".

Defensive size safety fixes in RDB and SDS

Layer / File(s)	Summary
RDB string length overflow guard src/rdb.c	rdbGenericLoadStringObject now rejects decoded lengths exceeding SIZE_MAX, reporting the RDB as corrupt and returning NULL.
SDS sdsIncrLen assertion standardization src/sds.c	Adds static inline sdsIncrLenAbs(ssize_t) and updates all SDS type branches in sdsIncrLen to use it and (size_t)incr for consistent negative/positive increment bounds assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: limiting multibulk arguments and fixing integer type truncations across multiple files.
Description check	✅ Passed	The description clearly relates to the changeset, detailing the specific safety improvements including PROTO_MAX_MULTIBULK_LEN, allocation error handling, type fixes, and RDB guards.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/sds.c (1)

464-464: The assertion is functionally correct, but consider standardizing the condition for consistency.

The SDS_TYPE_5 assertion uses incr > 0 while SDS_TYPE_8/16/32/64 use incr >= 0. Verification shows sdsIncrLen is never called with incr == 0 (all calls use positive or negative values), so the inconsistency doesn't cause a functional issue.

However, the > 0 vs >= 0 difference remains a code clarity concern. If standardizing the conditions is desired, change line 464 to use incr >= 0 to match the other types. This is optional, as the current code works correctly in practice.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/sds.c` at line 464, The assertion at line 464 for SDS_TYPE_5 uses the
condition `incr > 0` while the assertions for other SDS types
(SDS_TYPE_8/16/32/64) use `incr >= 0`, creating an inconsistency in code
clarity. To standardize the conditions across all types, modify the assertion at
line 464 to change the `incr > 0` condition to `incr >= 0` to match the pattern
used in the other SDS type assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/sds.c`:
- Line 464: The assertion at line 464 for SDS_TYPE_5 uses the condition `incr >
0` while the assertions for other SDS types (SDS_TYPE_8/16/32/64) use `incr >=
0`, creating an inconsistency in code clarity. To standardize the conditions
across all types, modify the assertion at line 464 to change the `incr > 0`
condition to `incr >= 0` to match the pattern used in the other SDS type
assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2fde5a94-a40b-4a2a-bf7d-79fa770cf1be

📥 Commits

Reviewing files that changed from the base of the PR and between 436dcae and e181997.

📒 Files selected for processing (5)

• src/networking.c
• src/rdb.c
• src/sds.c
• src/server.h
• tests/unit/protocol.tcl