Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions store/memory_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package store
import (
"bytes"
"context"
"encoding/binary"
"encoding/gob"
"hash/crc32"
"io"
"log/slog"
"os"
Expand Down Expand Up @@ -209,22 +211,38 @@ func (s *memoryStore) Snapshot() (io.ReadWriter, error) {
s.mtx.RUnlock()

buf := &bytes.Buffer{}
err := gob.NewEncoder(buf).Encode(cl)
if err != nil {
if err := gob.NewEncoder(buf).Encode(cl); err != nil {
return nil, errors.WithStack(err)
}

sum := crc32.ChecksumIEEE(buf.Bytes())
if err := binary.Write(buf, binary.LittleEndian, sum); err != nil {
return nil, errors.WithStack(err)
}

return buf, nil
}
func (s *memoryStore) Restore(buf io.Reader) error {
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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
Magic number: 4, in detected (mnd)

return errors.WithStack(ErrInvalidChecksum)
}
payload := data[:len(data)-4]
expected := binary.LittleEndian.Uint32(data[len(data)-4:])
if crc32.ChecksumIEEE(payload) != expected {
Comment on lines +225 to +238

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 👍 / 👎.

return errors.WithStack(ErrInvalidChecksum)
}

s.m = make(map[uint64][]byte)
if err := gob.NewDecoder(bytes.NewReader(payload)).Decode(&s.m); err != nil {
return errors.WithStack(err)
}

return nil
}
Expand Down
41 changes: 41 additions & 0 deletions store/memory_store_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package store

import (
"bytes"
"context"
"strconv"
"sync"
Expand Down Expand Up @@ -204,6 +205,46 @@ func TestMemoryStore_TTL(t *testing.T) {
wg.Wait()
}

func TestMemoryStore_SnapshotChecksum(t *testing.T) {
t.Parallel()

t.Run("success", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
st := NewMemoryStore()
assert.NoError(t, st.Put(ctx, []byte("foo"), []byte("bar")))

buf, err := st.Snapshot()
assert.NoError(t, err)

st2 := NewMemoryStore()
err = st2.Restore(bytes.NewReader(buf.(*bytes.Buffer).Bytes()))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
right hand must be only type assertion (forcetypeassert)

assert.NoError(t, err)

v, err := st2.Get(ctx, []byte("foo"))
assert.NoError(t, err)
assert.Equal(t, []byte("bar"), v)
})

t.Run("corrupted", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
st := NewMemoryStore()
assert.NoError(t, st.Put(ctx, []byte("foo"), []byte("bar")))

buf, err := st.Snapshot()
assert.NoError(t, err)
data := buf.(*bytes.Buffer).Bytes()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
right hand must be only type assertion (forcetypeassert)

corrupted := make([]byte, len(data))
copy(corrupted, data)
corrupted[0] ^= 0xff

st2 := NewMemoryStore()
err = st2.Restore(bytes.NewReader(corrupted))
assert.ErrorIs(t, err, ErrInvalidChecksum)
})
}

func TestMemoryStore_TTL_Txn(t *testing.T) {
ctx := context.Background()
t.Parallel()
Expand Down
36 changes: 31 additions & 5 deletions store/rb_memory_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package store
import (
"bytes"
"context"
"encoding/binary"
"encoding/gob"
"hash/crc32"
"io"
"log/slog"
"os"
Expand Down Expand Up @@ -229,22 +231,46 @@ func (s *rbMemoryStore) Snapshot() (io.ReadWriter, error) {
s.mtx.RUnlock()

buf := &bytes.Buffer{}
err := gob.NewEncoder(buf).Encode(cl)
if err != nil {
if err := gob.NewEncoder(buf).Encode(cl); err != nil {
return nil, errors.WithStack(err)
}

sum := crc32.ChecksumIEEE(buf.Bytes())
if err := binary.Write(buf, binary.LittleEndian, sum); err != nil {
return nil, errors.WithStack(err)
}

return buf, nil
}
func (s *rbMemoryStore) Restore(buf io.Reader) error {
func (s *rbMemoryStore) Restore(r io.Reader) error {
s.mtx.Lock()
defer s.mtx.Unlock()

s.tree.Clear()
err := gob.NewDecoder(buf).Decode(&s.tree)
data, err := io.ReadAll(r)
if err != nil {
return errors.WithStack(err)
}
if len(data) < 4 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
Magic number: 4, in detected (mnd)

return errors.WithStack(ErrInvalidChecksum)
}
payload := data[:len(data)-4]
expected := binary.LittleEndian.Uint32(data[len(data)-4:])
if crc32.ChecksumIEEE(payload) != expected {
return errors.WithStack(ErrInvalidChecksum)
}

var cl map[*[]byte][]byte
if err := gob.NewDecoder(bytes.NewReader(payload)).Decode(&cl); err != nil {
return errors.WithStack(err)
}

s.tree.Clear()
for k, v := range cl {
if k == nil {
continue
}
s.tree.Put(*k, v)
}

return nil
}
Expand Down
41 changes: 41 additions & 0 deletions store/rb_memory_store_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package store

import (
"bytes"
"context"
"encoding/binary"
"strconv"
Expand Down Expand Up @@ -205,6 +206,46 @@ func TestRbMemoryStore_Txn(t *testing.T) {
})
}

func TestRbMemoryStore_SnapshotChecksum(t *testing.T) {
t.Parallel()

t.Run("success", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
st := NewRbMemoryStore()
assert.NoError(t, st.Put(ctx, []byte("foo"), []byte("bar")))

buf, err := st.Snapshot()
assert.NoError(t, err)

st2 := NewRbMemoryStore()
err = st2.Restore(bytes.NewReader(buf.(*bytes.Buffer).Bytes()))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
right hand must be only type assertion (forcetypeassert)

assert.NoError(t, err)

v, err := st2.Get(ctx, []byte("foo"))
assert.NoError(t, err)
assert.Equal(t, []byte("bar"), v)
})

t.Run("corrupted", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
st := NewRbMemoryStore()
assert.NoError(t, st.Put(ctx, []byte("foo"), []byte("bar")))

buf, err := st.Snapshot()
assert.NoError(t, err)
data := buf.(*bytes.Buffer).Bytes()
corrupted := make([]byte, len(data))
copy(corrupted, data)
corrupted[0] ^= 0xff

st2 := NewRbMemoryStore()
err = st2.Restore(bytes.NewReader(corrupted))
assert.ErrorIs(t, err, ErrInvalidChecksum)
})
}

func TestRbMemoryStore_TTL(t *testing.T) {
ctx := context.Background()
t.Parallel()
Expand Down
1 change: 1 addition & 0 deletions store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
var ErrKeyNotFound = errors.New("not found")
var ErrUnknownOp = errors.New("unknown op")
var ErrNotSupported = errors.New("not supported")
var ErrInvalidChecksum = errors.New("invalid checksum")

type KVPair struct {
Key []byte
Expand Down
Loading