Fix HLL Validation in CopyUpdate path

[kevin-montrose]

It's currently possible to confuse PFADD into operating on an invalid hyperloglog record if the record is in the immutable region.

This fix that bug, and adds tests for PFADD and PFMERGE (which isn't affected, but could very well have been). Tests cover both inline and overflow records.

It also hardens hyperloglog validation in general by removing some length-less checks and adding an explicit length check during header validation.

[Copilot]

🟡 Changes recommended

PFADD still reads unvalidated record bytes during destination sizing before the new copy-update check runs.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes PFADD validation for immutable HyperLogLog records and strengthens length-aware header validation.

Changes:

• Validates HLL records during PFADD copy updates.
• Adds mutable/immutable validation tests for PFADD and PFMERGE.
• Adds a minimum-length check before reading the HLL signature.

File summaries

File	Description
RMWMethods.cs	Validates source HLL data during copy updates.
HyperLogLog.cs	Makes HLL signature validation length-aware.
HyperLogLogTests.cs	Tests invalid inline and overflow records before and after eviction.

Review details

Suppressed comments (1)

test/standalone/Garnet.test.complexstring/HyperLogLogTests.cs:1530

• DEBUG FLUSHANDEVICT moves these records out of the mutable region, so this comment states the opposite of the test setup.

            // Force into mutable region

• Files reviewed: 3/3 changed files
• Comments generated: 3
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Validation still occurs after unsafe sizing, and the PFMERGE test does not exercise an invalid destination.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

libs/server/Storage/Functions/MainStore/RMWMethods.cs:1221

• This validation runs after Tsavorite has already called GetRMWModifiedFieldInfo to size the copy destination. For PFADD, that path calls UpdateGrow, which reads the sparse RLE length at offset 16 without checking the source length. An immutable raw value shorter than 18 bytes with byte 3 set to the sparse type can therefore be read past its logical value before reaching this guard. Reject it in NeedCopyUpdate before sizing, or make UpdateGrow length-aware, and cover a sub-18-byte value.

                        if (!HyperLogLog.DefaultHLL.IsValidHYLL(srcLogRecord.PinnedValuePointer, srcLogRecord.ValueSpan.Length))

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.