Skip to content

Commit 894d2f9

Browse files
committed
fix(raftengine): PR783 r3 claude — reorder scanner guards to honor empty-range contract
Claude r3 on PR #783 flagged a contract violation in HasEncryptionRelevantEntryInRange's guard ordering: The EncryptionRelevantScanner interface (audit.go) requires the empty-range case (startExclusive >= endInclusive) to return (false, nil) UNCONDITIONALLY. The previous implementation checked s.storage == nil FIRST, so a nil-storage scanner called with an empty range would return the nil-storage error instead of the contract-required (false, nil). GuardSidecarBehindRaftLog in audit.go pre-filters the empty-gap case (`if sidecarAppliedIdx >= engineAppliedIdx { return nil }`) so this branch is unreachable in production today. But future callers (6C-2d main.go phase, 6D capability fan-out) might defensively call scan with a same-index range on a maybe-nil engine. Pinning the order now avoids reintroducing the surprise later. Fix: reorder so empty-range short-circuits before nil-storage. New TestEncryptionScanner_NilStorageEmptyRange pins the order against regressions; the existing TestEncryptionScanner_NilStorageError still passes because it uses a non-empty range (0, 5), so the empty-range short-circuit doesn't fire first. ## Verification - go test -race -timeout=60s ./internal/raftengine/etcd/... — PASS - golangci-lint run ./internal/raftengine/etcd/... — 0 issues
1 parent 644e428 commit 894d2f9

2 files changed

Lines changed: 35 additions & 5 deletions

File tree

internal/raftengine/etcd/encryption_scanner.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,22 @@ type encryptionScanner struct {
5555
// caller precomputes the non-empty-gap branch and only calls scan
5656
// when there's a range to inspect, but the defensive guard stays.
5757
func (s *encryptionScanner) HasEncryptionRelevantEntryInRange(startExclusive, endInclusive uint64) (bool, error) {
58-
if s.storage == nil {
59-
return false, errors.New("encryption scanner: storage is nil (engine not opened?)")
60-
}
58+
// The EncryptionRelevantScanner contract (audit.go) requires
59+
// the empty-range case to return (false, nil) UNCONDITIONALLY
60+
// — including on nil-storage receivers. Reorder the guards so
61+
// an empty range short-circuits before any state-dependent
62+
// failure path. GuardSidecarBehindRaftLog pre-filters the
63+
// empty-gap case in production, so this is contract conformance
64+
// rather than a reachable bug; pinning the order keeps any
65+
// future caller (6C-2d, capability fan-out) from triggering
66+
// the surprising "empty range on nil scanner returns error"
67+
// path.
6168
if startExclusive >= endInclusive {
6269
return false, nil
6370
}
71+
if s.storage == nil {
72+
return false, errors.New("encryption scanner: storage is nil (engine not opened?)")
73+
}
6474
// MemoryStorage.Entries(lo, hi, ...) returns indices [lo, hi).
6575
// We want (startExclusive, endInclusive] = [startExclusive+1,
6676
// endInclusive+1). Guard against uint64 wraparound at

internal/raftengine/etcd/encryption_scanner_test.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,32 @@ func TestEncryptionScanner_CompactedRange(t *testing.T) {
238238
// TestEncryptionScanner_NilStorageError verifies the defensive
239239
// nil-storage check. A zero-value encryptionScanner (or one
240240
// constructed before the engine is opened) must fail closed
241-
// rather than nil-deref.
241+
// rather than nil-deref. The range is non-empty so the
242+
// empty-range short-circuit doesn't fire first.
242243
func TestEncryptionScanner_NilStorageError(t *testing.T) {
243244
var s encryptionScanner
244245
_, err := s.HasEncryptionRelevantEntryInRange(0, 5)
245246
if err == nil {
246-
t.Fatal("nil-storage scanner must return an error")
247+
t.Fatal("nil-storage scanner must return an error on a non-empty range")
248+
}
249+
}
250+
251+
// TestEncryptionScanner_NilStorageEmptyRange pins the
252+
// EncryptionRelevantScanner contract: empty range MUST return
253+
// (false, nil) UNCONDITIONALLY — including on a nil-storage
254+
// scanner. The guard order in HasEncryptionRelevantEntryInRange
255+
// is empty-range-first, then nil-storage, so this test pins the
256+
// ordering against future regressions where someone reorders the
257+
// guards and reintroduces the contract violation claude flagged
258+
// on PR #783 r3.
259+
func TestEncryptionScanner_NilStorageEmptyRange(t *testing.T) {
260+
var s encryptionScanner
261+
hit, err := s.HasEncryptionRelevantEntryInRange(5, 5)
262+
if err != nil {
263+
t.Fatalf("nil-storage scanner with empty range MUST return (false, nil) per contract; got err=%v", err)
264+
}
265+
if hit {
266+
t.Fatalf("empty range must report no hit; got hit=%v", hit)
247267
}
248268
}
249269

0 commit comments

Comments
 (0)