chore: harden listpack checks in t_stream code #6440

romange · 2026-01-21T16:23:04Z

No description provided.

Copilot

Pull request overview

This PR renames and enhances the listpack validation function in the Redis stream implementation, adding more comprehensive checks to detect corrupted listpacks earlier in stream operations.

Changes:

Renamed checkListPackNotEmpty to checkListPackValid to better reflect expanded validation scope
Added validation for corrupted listpack headers (zero-byte listpacks)
Added validation using lpValidateFirst and lpValidateNext to check listpack integrity
Applied the enhanced validation at three call sites in stream trimming and entry removal operations

Copilot · 2026-01-21T16:25:52Z

src/redis/t_stream.c

+   fprintf(stderr, "Error: corrupted listpack header found.");
+   abort();
+  }
+  p = lpValidateFirst(lp);
+  if(p[0] == 0xFF) {
+   fprintf(stderr, "Error: empty listpack found.");
+   abort();
+  }
+
+  if (lpValidateNext(lp, &p, lpBytes(lp)) == 0) {
+   lpRepr(lp);
   fprintf(stderr, "Error: corrupted listpack found.");


Missing newline character in error message. The error messages should end with '\n' to ensure proper formatting when written to stderr. This applies to the error messages on lines 284, 289, and 295.

Suggested change

fprintf(stderr, "Error: corrupted listpack header found.");

abort();

}

p = lpValidateFirst(lp);

if(p[0] == 0xFF) {

fprintf(stderr, "Error: empty listpack found.");

abort();

}

if (lpValidateNext(lp, &p, lpBytes(lp)) == 0) {

lpRepr(lp);

fprintf(stderr, "Error: corrupted listpack found.");

fprintf(stderr, "Error: corrupted listpack header found.\n");

abort();

}

p = lpValidateFirst(lp);

if(p[0] == 0xFF) {

fprintf(stderr, "Error: empty listpack found.\n");

abort();

}

if (lpValidateNext(lp, &p, lpBytes(lp)) == 0) {

lpRepr(lp);

fprintf(stderr, "Error: corrupted listpack found.\n");

Copilot · 2026-01-21T16:25:53Z

src/redis/t_stream.c

  if(lpBytes(lp) == 0) {
+   fprintf(stderr, "Error: corrupted listpack header found.");
+   abort();
+  }
+  p = lpValidateFirst(lp);
+  if(p[0] == 0xFF) {
+   fprintf(stderr, "Error: empty listpack found.");
+   abort();
+  }
+
+  if (lpValidateNext(lp, &p, lpBytes(lp)) == 0) {


Inconsistent code style: missing spaces after 'if' keyword. The code should follow consistent formatting with space after 'if', like 'if (' instead of 'if(' as seen on lines 283, 288, and 293.

src/redis/t_stream.c

augmentcode · 2026-01-21T16:26:23Z

🤖 Augment PR Summary

Summary: Hardens stream listpack validation in t_stream.c by replacing checkListPackNotEmpty with a more comprehensive checkListPackValid.
Changes: Adds header/emptiness checks and invokes listpack entry validation before reading or reinserting listpacks during stream trimming and iterator entry removal.

_{🤖 Was this summary useful? React with 👍 or 👎}

augmentcode

Review completed. 1 suggestions posted.

Comment augment review to trigger a new review at any time.

src/redis/t_stream.c

Signed-off-by: Roman Gershman <[email protected]>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Copilot · 2026-01-21T16:35:29Z

src/redis/t_stream.c

+void checkListPackValid(unsigned char* lp) {
+  unsigned char* p = NULL;
  if(lpBytes(lp) == 0) {
-   fprintf(stderr, "Error: corrupted listpack found.");
+   fprintf(stderr, "Error: corrupted listpack header found.\n");
+   abort();
+  }
+  p = lpValidateFirst(lp);
+  if(p == NULL) {
+   fprintf(stderr, "Error: empty listpack found.\n");
+   abort();
+  }
+
+  if (lpValidateNext(lp, &p, lpBytes(lp)) == 0) {
+   lpRepr(lp);
+   fprintf(stderr, "Error: corrupted listpack found.\n");
   abort();
  }
 }


The validation logic only checks the first entry of the listpack, not the entire structure. After lpValidateFirst returns a pointer to the first element, lpValidateNext validates that element and advances the pointer to the next one, but doesn't continue validating subsequent entries. This means corrupted data in entries after the first one would not be detected. Consider validating all entries or at least a representative sample, similar to how streamValidateListpackIntegrity does it.

Copilot · 2026-01-21T16:35:29Z

src/redis/t_stream.c

+void checkListPackValid(unsigned char* lp) {
+  unsigned char* p = NULL;
  if(lpBytes(lp) == 0) {
-   fprintf(stderr, "Error: corrupted listpack found.");
+   fprintf(stderr, "Error: corrupted listpack header found.\n");
+   abort();
+  }
+  p = lpValidateFirst(lp);
+  if(p == NULL) {
+   fprintf(stderr, "Error: empty listpack found.\n");
+   abort();
+  }
+
+  if (lpValidateNext(lp, &p, lpBytes(lp)) == 0) {
+   lpRepr(lp);
+   fprintf(stderr, "Error: corrupted listpack found.\n");
   abort();
  }


Inconsistent code formatting: the function body uses 2-space indentation while the surrounding code uses 4-space indentation. Missing space after if keywords on lines 283 and 288 (should be if ( not if().

Copilot · 2026-01-21T16:35:30Z

src/redis/t_stream.c


-void checkListPackNotEmpty(unsigned char* lp) {
+void checkListPackValid(unsigned char* lp) {
+  unsigned char* p = NULL;


The variable p is initialized to NULL on line 282 but this initialization is unnecessary since it's immediately reassigned on line 287. Consider removing the initialization or declaring the variable where it's first assigned.

Suggested change

unsigned char* p = NULL;

unsigned char* p;

romange · 2026-01-22T05:49:15Z

I do not think we will need this PR

Copilot AI review requested due to automatic review settings January 21, 2026 16:23

Copilot started reviewing on behalf of romange January 21, 2026 16:23 View session

romange requested a review from kostasrim January 21, 2026 16:24

Copilot AI reviewed Jan 21, 2026

View reviewed changes

augmentcode bot reviewed Jan 21, 2026

View reviewed changes

src/redis/t_stream.c Outdated Show resolved Hide resolved

romange force-pushed the Pr6 branch from 8b806b9 to f4ff826 Compare January 21, 2026 16:28

chore: harden listpack checks in t_stream code

6c1900d

Signed-off-by: Roman Gershman <[email protected]>

Copilot AI review requested due to automatic review settings January 21, 2026 16:32

romange force-pushed the Pr6 branch from f4ff826 to 6c1900d Compare January 21, 2026 16:32

Copilot started reviewing on behalf of romange January 21, 2026 16:32 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

romange removed the request for review from kostasrim January 22, 2026 05:49

romange closed this Jan 30, 2026

romange deleted the Pr6 branch January 30, 2026 08:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: harden listpack checks in t_stream code #6440

chore: harden listpack checks in t_stream code #6440

romange commented Jan 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Uh oh!

augmentcode bot commented Jan 21, 2026

Uh oh!

augmentcode bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

romange commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chore: harden listpack checks in t_stream code #6440

chore: harden listpack checks in t_stream code #6440

Conversation

romange commented Jan 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

augmentcode bot commented Jan 21, 2026

Uh oh!

augmentcode bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

romange commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants