Cap HRANDFIELD/ZRANDMEMBER count before packing it into arg1

[hexonal (hexonal)]

HRANDFIELD and ZRANDMEMBER return the wrong thing for any count above 536,870,911. Against a 3-field hash and a 3-member sorted set:

HSET h f1 v1 f2 v2 f3 v3
ZADD z 1 m1 2 m2 3 m3

HRANDFIELD h 1073741824    -> *0                     expected all 3 fields
HRANDFIELD h 2147483647    -> a single field         expected all 3 fields
HRANDFIELD h 536870912     -> no reply for tens of seconds, then
                              -ERR Garnet Exception: Exceeded maximum response
                              size of (2,147,483,591) bytes
                                                     expected all 3 fields

ZRANDMEMBER z 1073741824   -> nothing at all is written; the next reply on the
                              connection answers this command
                                                     expected all 3 members
ZRANDMEMBER z 2147483647   -> a single member        expected all 3 members
ZRANDMEMBER z 536870912    -> same stall and error as HRANDFIELD
                                                     expected all 3 members

The ZRANDMEMBER key 1073741824 case is the worst of the six: the client blocks until the next command's reply arrives and then reads it as the answer to ZRANDMEMBER, so every subsequent reply on that connection is off by one. The stall cases hold the session for the duration of a ~2GB allocation. The session survives all six; nothing here kills a connection or throws OutOfMemoryException.

Cause

Both commands pack the user-supplied count above two metadata bits into the single int the object store receives as arg1:

• libs/server/Resp/Objects/HashCommands.cs:249
• libs/server/Resp/Objects/SortedSetCommands.cs:851

var countWithMetadata = (((paramCount << 1) | (includedCount ? 1 : 0)) << 1) | (withValues ? 1 : 0);

The shift is unchecked, so any count above int.MaxValue >> 2 (536,870,911) wraps. The storage side unwinds it with an arithmetic >> 2 at HashObjectImpl.cs:115 and SortedSetObjectImpl.cs:646 and gets an unrelated number. Three regimes:

• Unpacks to 0. Exactly four counts do this: -2147483648, -1073741824, 0 and 1073741824. 0 is short-circuited at the RESP layer, so the only positive count affected is 1073741824.
• Unpacks to a negative value. 2147483647 becomes -1, which the object layer reads as "1 element, repeats allowed". 536870912 becomes -536870912.
• Unpacks to a smaller positive value. For counts in [2^30, 1610612736) the result is count - 2^30.

The -536870912 case is what stalls: indexCount is Math.Abs(count), so HashObjectImpl.cs:140 and SortedSetObjectImpl.cs:665 allocate new int[536870912] (~2GB), fill it, and then try to write 536 million bulk strings, which trips the response-size ceiling.

The count-of-0 case only desynchronises for ZRANDMEMBER. HashObjectImpl.cs:146 writes the array header unconditionally, so HRANDFIELD emits *0. SortedSetObjectImpl.cs:659 writes it only under arrayLength > 1 || (arrayLength == 1 && includedCount), and a count of 0 satisfies neither, so no bytes are produced at all.

Fix

One line in each command, before the shift:

paramCount = Math.Min(paramCount, int.MaxValue >> 2);

536870911 * 4 + 3 == int.MaxValue exactly, so the cap value and both metadata bits round-trip with no room to spare. The object layer already saturates any positive count to the collection size (HashObjectImpl.cs:135, SortedSetObjectImpl.cs:652), so a capped count replies with the whole collection, which is what these six commands should have returned all along.

The in-repo sibling is SRANDMEMBER, which passes the count straight through as arg1 with no metadata bits (SetCommands.cs:655) and therefore has no equivalent overflow: SRANDMEMBER s 2147483647 already returns the whole set. This brings HRANDFIELD and ZRANDMEMBER in line with it.

Surface of the behaviour change:

• Math.Min is a no-op for every count at or below 536,870,911, so nothing below the cap moves.
• Above the cap, the sub-regime [2^30, 1610612736) unpacked to count - 2^30, and whenever that remainder already exceeded the collection size the reply was correct. Those replies are byte-identical after the cap, because both the old remainder and the new cap saturate to the same collection size. Every other count above the cap was wrong and is now right.
• The cap does mean a collection holding more than 536,870,911 elements would return 536,870,911 of them rather than all for a huge positive count. Such a reply is an order of magnitude past the 2,147,483,591-byte response ceiling, so it is not reachable.

Tests

CanDoHRANDFIELDWithOverflowingCountLC in RespHashTests.cs and CanUseZRandMemberWithOverflowingCount in RespSortedSetTests.cs. Both use LightClientRequest with a PING pipelined behind every command, so a short, missing or duplicated reply is caught rather than absorbed. StackExchange.Redis cannot observe the ZRANDMEMBER missing-reply case at all, which is why these are raw RESP.

Four assertions per test do not pass on unpatched main:

• count 1073741824
• count 536870912
• count 2147483647
• count 2147483647 with WITHVALUES / WITHSCORES

Seven per test are green regression guards that pass with or without the production change: count 536870911 (the boundary the cap must not move), 2, 5, -5, 0, no count at all, and 2 with WITHVALUES / WITHSCORES.

One note on how the red cases fail: LightClientRequest.CompletePendingRequests waits for a token count, so on unpatched main these four stall the request rather than raising an assertion. They do not pass; they do not fail fast either.

Garnet.test.collections: 750 passed, 0 failed. dotnet format --verify-no-changes clean.

Known gap, deliberately not fixed here

ZRANDMEMBER key -1073741824 and ZRANDMEMBER key -2147483648, with or without WITHSCORES, also unpack to 0 and so still write no reply at all. That is unchanged by this PR, and it is the same desynchronisation described above.

The cap cannot fix it. Capping the negative side at int.MinValue >> 2 (-536870912) sends those two inputs into the Math.Abs(count) index array instead, replacing an instant desync with a ~2GB allocation, a stall of tens of seconds and an Exceeded maximum response size error, on 1.6 billion inputs this change does not otherwise touch. I measured both shapes before settling on the one-sided cap.

The right fix is in the object layer, and on a closer look it is not the one-liner I first described. HRANDFIELD has an explicit early exit at HashObjectImpl.cs:127 — if (count == 0) { WriteEmptyArray(); return; }, commented "This can happen because of expiration but RMW operation haven't applied yet" — and SortedSetRandomMember has no equivalent. Giving it that same early exit closes three shapes at once: the overflow-to-0 case above, ZRANDMEMBER key 3 against a sorted set whose members have all expired but not yet been collected (Count() returns 0 while the object is still present, so the read returns OK and zero bytes are written), and ZRANDMEMBER key -3 on that same key, which currently writes an array header and then indexes an empty set. Merely relaxing the header guard at SortedSetObjectImpl.cs:659 would not cover the last two. I left it out to keep this change at the RESP layer and off a file two other open PRs touch, and because the expiration half is a live bug on main independent of any overflow — happy to fold it in here, or to file it separately, whichever you prefer.

HRANDFIELD has no equivalent gap: HashObjectImpl.cs:146 writes the array header unconditionally, so HRANDFIELD key -1073741824 and HRANDFIELD key -2147483648 reply *0 immediately, identical to main.

Separately, bringing negative counts fully in line with Redis, which streams them in batches instead of materialising an index array of |count| entries, is a larger change and out of scope here.

Environment

macOS arm64, net10.0 Debug. Reproductions driven over raw RESP against a local server on 127.0.0.1.

[Copilot]

Pull request overview

This PR fixes an integer overflow bug in the RESP-layer argument packing used by HRANDFIELD and ZRANDMEMBER when a large positive count is shifted into a single int alongside metadata bits. By capping the count to the largest value that can be round-tripped through the packed field, the commands no longer desynchronize connections (missing reply) or trigger huge allocations / response-size-limit errors for specific large counts.

Changes:

• Clamp paramCount to int.MaxValue >> 2 before packing it with metadata bits for HRANDFIELD and ZRANDMEMBER.
• Add LightClient-based regression tests that cover previously-broken large-count regimes and verify replies remain correctly framed when pipelined with PING.

Reviewed changes

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

File	Description
libs/server/Resp/Objects/SortedSetCommands.cs	Caps ZRANDMEMBER count prior to packing into arg1 to avoid Int32 overflow during bit packing.
libs/server/Resp/Objects/HashCommands.cs	Caps HRANDFIELD count prior to packing into arg1 to avoid Int32 overflow during bit packing.
test/standalone/Garnet.test.collections/RespSortedSetTests.cs	Adds LightClient regression test validating large-count ZRANDMEMBER responses are complete/framed under pipelining.
test/standalone/Garnet.test.collections/RespHashTests.cs	Adds LightClient regression test validating large-count HRANDFIELD responses are complete/framed under pipelining.

[kevin-montrose]

The change is mostly fine, the comment here is overkill and kinda confusing.

Change it (and the one in SortedSetCommands) to something like:

// We reserve to 2 bits of metadata (1 for "include count", 1 for "include scores") in ObjectInput.arg0, so cap count to what will fit in 30-bits.

[kevin-montrose]

This may be a problem with the existing code, but int instead of uint is strange here, seems like we're leaving a bit on the table.

Can you explore a max count of (int)(uint.MaxValue >> 2)? May require some changes in the *Impl.cs files

[hexonal (hexonal)]

Comments shortened to your wording (in both HashCommands and SortedSetCommands), and rebased onto current main. One small correction: the field is ObjectInput.arg1, not arg0, so that is what the comment names.

On exploring (int)(uint.MaxValue >> 2) — I don't think the bit is on the table. It's the sign bit, and count is genuinely signed: a negative count means "sample with repetition" for both commands, and HashObjectImpl/SortedSetObjectImpl use countParameter > 0 to pick the sampling mode after unpacking with an arithmetic >> 2.

Reaching a 30-bit magnitude requires unpacking with >>> instead, and then negative counts stop round-tripping. Packing ((count << 1) | includedCount) << 1 | withValues over the full 32 bits:

count	packed arg1	>> 2 (today)	>>> 2 (needed for 30-bit)
536870911 (int.MaxValue >> 2)	2147483647	536870911	536870911
1073741823 ((int)(uint.MaxValue >> 2))	-1	-1	1073741823
-1	-1	-1	1073741823
-3	-9	-3	1073741821
-536870912 (int.MinValue >> 2)	-2147483645	-536870912	536870912

HRANDFIELD key 1073741823 and HRANDFIELD key -1 pack to the same arg1, so no unpacking rule can separate them — the encoding is ambiguous above int.MaxValue >> 2, not merely truncating. Switching to >>> would fix the large-positive case by breaking every negative count, including the extremely common -1.

Getting 30 usable magnitude bits would mean carrying the sign somewhere other than arg1 (arg2 is the seed, and there is no third slot). That would buy nothing observable, though: both *Impl.cs files already saturate a positive count to the collection size, so a cap of ~536M vs ~1073M is indistinguishable unless a single hash or sorted set holds more than 536,870,911 elements. Happy to do it if you'd still like the range widened, but as it stands int.MaxValue >> 2 is exactly the largest representable count rather than an arbitrary limit.

[hexonal (hexonal)]

Rebased onto current main and fixed the PR description — every SortedSetObjectImpl.cs line number in it was stale by 6–8 lines, including the one I pointed you at for the proposed follow-up. It landed on the saturation check rather than the array-header guard, which would have made that paragraph read as nonsense. Corrected to 646 / 652 / 659 / 665.

I also corrected the follow-up proposal itself, because relaxing the header guard is not sufficient. HRANDFIELD has an explicit early exit that SortedSetRandomMember simply lacks:

// HashObjectImpl.cs:127
if (count == 0) // This can happen because of expiration but RMW operation haven't applied yet
{
    writer.WriteEmptyArray();
    output.result1 = 0;
    return;
}

Giving SortedSetRandomMember the same early exit closes three shapes at once rather than one: the overflow-to-0 case, ZRANDMEMBER key 3 against a sorted set whose members have all expired but not yet been collected (Count() returns 0 while the object is still present, so the read returns OK and zero bytes reach the wire), and ZRANDMEMBER key -3 on that same key, which writes an array header and then indexes an empty set. The last two are live on main today and have nothing to do with the overflow this PR caps. Happy to fold it in here or file it separately — your call.

On the red check: Garnet Standalone (ubuntu-latest, net10.0, Release, Garnet.test.scripting) is not this PR. The two failures are LuaScriptTests.Bit and LuaScriptTests.CanDoEvalSetGet (ERR Lua encountered an error: bad argument to tobit); this PR touches only HashCommands.cs / SortedSetCommands.cs comments plus a Math.Min, and the log has zero mentions of HRANDFIELD or ZRANDMEMBER. I ran Garnet.test.scripting locally on this exact commit in Release: 435 passed, 0 failed, 15 skipped.

Separately, while triaging a different red leg on #2050 I traced a process-level FailFast in the object allocator's shutdown path and filed it as #2069 — worth knowing about, since that signature can abort an unrelated test run mid-suite from 2026-08-12 onward.