Skip to content

Validate slot import ranges in valkey-check-rdb when checking slot RDB - #4470

Open
enjoy-binbin wants to merge 1 commit into
valkey-io:unstablefrom
enjoy-binbin:check_rdb
Open

Validate slot import ranges in valkey-check-rdb when checking slot RDB#4470
enjoy-binbin wants to merge 1 commit into
valkey-io:unstablefrom
enjoy-binbin:check_rdb

Conversation

@enjoy-binbin

Copy link
Copy Markdown
Member

The runtime RDB load path (clusterRDBLoadSlotImport) already rejects
slot import ranges where start/end >= CLUSTER_SLOTS or start > end, but
valkey-check-rdb only read the lengths without validating them.

Add the same range validation so check-rdb can detect such malformed RDB
files instead of silently passing them.

Follow up to #4229.

The runtime RDB load path (clusterRDBLoadSlotImport) already rejects
slot import ranges where start/end >= CLUSTER_SLOTS or start > end, but
valkey-check-rdb only read the lengths without validating them.

Add the same range validation so check-rdb can detect such malformed RDB
files instead of silently passing them.

Follow up to valkey-io#4229.

Signed-off-by: Binbin <binloveplay1314@qq.com>
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f0268f5-4aa6-41a6-a6b1-54538b83b088

📥 Commits

Reviewing files that changed from the base of the PR and between 36aca4b and a9cd9ed.

📒 Files selected for processing (1)
  • src/valkey-check-rdb.c

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

RDB slot-import parsing now stores both range endpoints, validates them against CLUSTER_SLOTS, and rejects ranges where the start exceeds the end.

Changes

RDB slot-import validation

Layer / File(s) Summary
Slot range validation
src/valkey-check-rdb.c
The parser retains both slot endpoints, validates their bounds and ordering, and reports an RDB error for invalid ranges.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to a9cd9

This localized change adds validation for malformed slot import ranges in RDB checking, preventing invalid files from passing silently; no actionable merge-blocking risk remains.

Possibly related PRs

Suggested reviewers: murphyjacob4

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the slot import range validation added to valkey-check-rdb.
Description check ✅ Passed The description accurately explains the validation change, its purpose, and its alignment with the runtime RDB load path.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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

Comment @coderabbitai help to get the list of available commands.

@valkey-review-bot valkey-review-bot Bot 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.

The new malformed-range rejection path needs targeted regression coverage in the existing slot-import RDB test.

Comment thread src/valkey-check-rdb.c
uint64_t start_slot, end_slot;
if ((start_slot = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr;
if ((end_slot = rdbLoadLen(rdb, NULL)) == RDB_LENERR) goto eoferr;
if (start_slot >= CLUSTER_SLOTS || end_slot >= CLUSTER_SLOTS || start_slot > end_slot) {

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.

tests/integration/rdb-slot-import.tcl already crafts RDB_OPCODE_SLOT_IMPORT records and runs VALKEY_CHECK_RDB_BIN, but this PR adds no case that reaches this check. Extend that fixture with start_slot == CLUSTER_SLOTS, end_slot == CLUSTER_SLOTS, and start_slot > end_slot, asserting each reports Invalid slot import range and does not report RDB looks OK.

@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.78%. Comparing base (413f6c3) to head (a9cd9ed).
⚠️ Report is 2 commits behind head on unstable.

Files with missing lines Patch % Lines
src/valkey-check-rdb.c 0.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #4470      +/-   ##
============================================
+ Coverage     78.73%   78.78%   +0.05%     
============================================
  Files           170      170              
  Lines         89783    89794      +11     
============================================
+ Hits          70692    70748      +56     
+ Misses        19091    19046      -45     
Files with missing lines Coverage Δ
src/valkey-check-rdb.c 72.34% <0.00%> (-0.52%) ⬇️

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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