Add checksum validation to store snapshots #221
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| assert.NoError(t, err) | ||
|
|
||
| st2 := NewMemoryStore() | ||
| err = st2.Restore(bytes.NewReader(buf.(*bytes.Buffer).Bytes())) |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
right hand must be only type assertion (forcetypeassert)
|
|
||
| buf, err := st.Snapshot() | ||
| assert.NoError(t, err) | ||
| data := buf.(*bytes.Buffer).Bytes() |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
right hand must be only type assertion (forcetypeassert)
| assert.NoError(t, err) | ||
|
|
||
| st2 := NewRbMemoryStore() | ||
| err = st2.Restore(bytes.NewReader(buf.(*bytes.Buffer).Bytes())) |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
right hand must be only type assertion (forcetypeassert)
| if err != nil { | ||
| return errors.WithStack(err) | ||
| } | ||
| if len(data) < 4 { |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
Magic number: 4, in detected (mnd)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| func (s *memoryStore) Restore(r io.Reader) error { | ||
| s.mtx.Lock() | ||
| defer s.mtx.Unlock() | ||
|
|
||
| s.m = make(map[uint64][]byte) | ||
| err := gob.NewDecoder(buf).Decode(&s.m) | ||
| data, err := io.ReadAll(r) | ||
| if err != nil { | ||
| return errors.WithStack(err) | ||
| } | ||
| if len(data) < 4 { | ||
| return errors.WithStack(ErrInvalidChecksum) | ||
| } | ||
| payload := data[:len(data)-4] | ||
| expected := binary.LittleEndian.Uint32(data[len(data)-4:]) | ||
| if crc32.ChecksumIEEE(payload) != expected { |
There was a problem hiding this comment.
[P1] Preserve ability to restore pre-checksum snapshots
The new Restore path now unconditionally expects the last 4 bytes of the snapshot to be a CRC32 checksum. Snapshots that were produced by previous releases contain only gob data, so upgrading a node with existing snapshot files will cause Restore to immediately return ErrInvalidChecksum and leave the store empty. This breaks backward compatibility and can prevent data recovery after deploys. Consider detecting the old format (e.g., fall back to gob decoding when the checksum validation fails or when the payload cannot decode) so that pre-existing snapshots still load. The same concern applies to rbMemoryStore.Restore.
Useful? React with 👍 / 👎.
| if err != nil { | ||
| return errors.WithStack(err) | ||
| } | ||
| if len(data) < 4 { |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
Magic number: 4, in detected (mnd)
Summary
ErrInvalidChecksumfor detecting corrupt snapshot dataTesting
go test ./...https://chatgpt.com/codex/tasks/task_e_68b1f3ef3e388324973d28a4bed306ea