Skip to content

Fix LRANGE returning the head element when stop is more negative than the list length - #2029

Merged
kevin-montrose merged 3 commits into
microsoft:mainfrom
hexonal:fix-lrange-negative-stop-clamp
Aug 7, 2026
Merged

Fix LRANGE returning the head element when stop is more negative than the list length#2029
kevin-montrose merged 3 commits into
microsoft:mainfrom
hexonal:fix-lrange-negative-stop-clamp

Conversation

@hexonal

Copy link
Copy Markdown
Contributor

LRANGE over a range whose stop is more negative than the list length returns the first element instead of an empty array.

$ redis-cli RPUSH lk a b c
(integer) 3

$ redis-cli LRANGE lk 0 -4
1) "a"            # Garnet; Redis replies (empty array)

$ redis-cli LRANGE lk 0 -5
1) "a"            # Garnet; Redis replies (empty array)

LRANGE lk 0 -3 is the last range that still addresses an element (a). Anything past it addresses nothing and should come back empty.

Root cause

libs/server/Objects/List/ListObjectImpl.cs:147

start = start < 0 ? list.Count + start : start;
if (start < 0) start = 0;

stop = stop < 0 ? list.Count + stop : stop;
if (stop < 0) stop = 0;                          // <-- this line
if (stop >= list.Count) stop = list.Count - 1;

if (start > stop || 0 == list.Count)

A stop that is still negative after adding list.Count addresses nothing. Clamping it to 0 turns it into the head index, and since start was already clamped to 0 two lines above, the start > stop guard no longer fires. The range collapses to [0, 0] and one element is written.

Fix

Delete the clamp. start is guaranteed >= 0, so leaving stop negative lets the existing start > stop guard fire and write the empty array.

This matches how sorted sets already do it: ZRANGE by index in libs/server/Objects/SortedSet/SortedSetObjectImpl.cs:478-500 normalizes a negative maxIndex without clamping it to 0, and clamps minIndex to 0 only after its (minIndex < 0 && maxIndex < 0) || (minIndex > maxIndex) emptiness check has run. ListTrim in the same file (ListObjectImpl.cs:185) likewise keeps end < 0 as its own empty-range term rather than clamping it away.

The stop >= list.Count clamp stays where it is, above the start > stop check: unlike Redis, Garnet's guard carries no start >= list.Count term, so that clamp is what makes LRANGE lk 5 100 empty.

Tests

Extended CanDoLRANGEcorrect in test/standalone/Garnet.test.collections/RespListTests.cs.

Fail on main today, pass with the fix — each returns ["a"] where an empty array is expected:

  • LRANGE key3 0 -4, LRANGE key3 0 -5, LRANGE key3 -5 -4 on a 3-element list
  • LRANGE key5 0 -6, LRANGE key5 0 -7 on a 5-element list

Already pass on main, added as regression guards for the boundary on either side:

  • LRANGE key3 0 -3 -> ["a"], the last stop that still addresses an element
  • LRANGE key3 1 -10 -> empty, which main gets right only by accident because start = 1 exceeded the clamped stop = 0

Existing coverage that pins the clamp order stays green: CanDoLRANGEbasic asserts LRANGE key -100 100 -> 3 elements and LRANGE key 5 100 -> empty on a 3-element list, and CanReturnEmptyArrayinListLC asserts LRANGE mylist 5 10 -> *0\r\n on the wire.

Copilot AI lite review requested due to automatic review settings August 6, 2026 09:21

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

Fixes Garnet’s LRANGE index normalization for lists so that ranges with a stop that remains negative after adding the list length correctly return an empty array (matching Redis behavior), instead of incorrectly returning the head element.

Changes:

  • Removed the stop < 0 clamp in ListObjectImpl.ListRange so the existing start > stop guard can return an empty array for out-of-range negative stop values.
  • Extended CanDoLRANGEcorrect with regression cases covering stop values more negative than -listLength (and nearby boundaries).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
libs/server/Objects/List/ListObjectImpl.cs Adjusts LRANGE stop-index normalization to avoid clamping still-negative stop to 0, allowing empty-range detection to work.
test/standalone/Garnet.test.collections/RespListTests.cs Adds regression tests ensuring LRANGE returns an empty array for overly-negative stop values on 3- and 5-element lists.

@kevin-montrose kevin-montrose self-assigned this Aug 6, 2026
@hexonal
hexonal (hexonal) force-pushed the fix-lrange-negative-stop-clamp branch from 54ba508 to c22d7f0 Compare August 7, 2026 05:25
@kevin-montrose
kevin-montrose merged commit 3cc5544 into microsoft:main Aug 7, 2026
316 of 317 checks passed
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.

3 participants