Skip to content

Commit 58db40c

Browse files
committed
raftengine: document header-error-before-CRC ordering in applyHeaderStateOnSkip (claude #934 R1-F1)
R1-F1 was the only open finding across 8 review rounds, marked doc- only with no safety impact. Add a comment in applyHeaderStateOnSkip spelling out why header parse runs before the body-CRC compare: - Both header-parse errors and CRC errors are equally fatal for the skip path (snapshot file corruption) and both must propagate without ever calling ApplySnapshotHeader. - CRC compare stays AFTER the header parse so the TeeReader has actually been drained when h.Sum32() is read; inverting the order would let a CRC mismatch surface on a truncated body even when the header was valid, muddying the operator-facing diagnostic. - Matches the same ordering openAndRestoreFSMSnapshot uses, so fallback paths see consistent error chains regardless of which routine ran. No code change; this is the only safe artifact of an audit comment. Tests pass; lint clean.
1 parent fc55c7b commit 58db40c

1 file changed

Lines changed: 14 additions & 0 deletions

File tree

internal/raftengine/etcd/wal_store.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,20 @@ func applyHeaderStateOnSkip(fsm StateMachine, snapPath string, tokenCRC uint32)
535535
// and hand it to the FSM's ParseSnapshotHeader for header parse
536536
// + drain. Every payload byte flows through h, matching
537537
// restoreAndComputeCRC's boundary in openAndRestoreFSMSnapshot.
538+
//
539+
// Error-ordering contract (claude #934 R1-F1): header parse
540+
// errors surface BEFORE the body-CRC compare runs, so callers
541+
// (the skip-gate fallback in restoreSnapshotState) may observe
542+
// either an ErrSnapshotHeaderUnknownMagic / InvalidLength chain or
543+
// an ErrFSMSnapshotFileCRC chain depending on which check fails
544+
// first. This is the same ordering openAndRestoreFSMSnapshot has
545+
// — both errors are equally fatal for the skip path (they signal
546+
// snapshot file corruption) and both must propagate without ever
547+
// calling ApplySnapshotHeader. The CRC check stays AFTER the
548+
// header parse so the TeeReader has actually been drained before
549+
// we read h.Sum32(); inverting the order would let a CRC mismatch
550+
// surface on a truncated body even when the header was valid,
551+
// muddying the operator-facing diagnostic.
538552
if _, err := file.Seek(0, io.SeekStart); err != nil {
539553
return errors.WithStack(err)
540554
}

0 commit comments

Comments
 (0)