Skip to content

Commit 6d7f722

Browse files
committed
fix #3231 prevent snapshot corruption
Clone raft Snapshot so that re-slicing Snapshot.Data does not mutate the initial Snapshot.Data through the shared pointer. Signed-off-by: Sylvere Richard <sylvere.richard@gmail.com>
1 parent 17b8d22 commit 6d7f722

2 files changed

Lines changed: 49 additions & 0 deletions

File tree

manager/state/raft/transport/peer.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ func splitSnapshotData(_ context.Context, m *raftpb.Message) []api.StreamRaftMes
172172
}
173173

174174
raftMsg := *m
175+
// Clone Snapshot so that re-slicing Snapshot.Data below
176+
// does not mutate m.Snapshot.Data through the shared pointer.
177+
snap := *m.Snapshot
178+
raftMsg.Snapshot = &snap
175179

176180
// sub-slice for this snapshot chunk.
177181
raftMsg.Snapshot.Data = m.Snapshot.Data[snapDataIndex : snapDataIndex+chunkSize]

manager/state/raft/transport/transport_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,51 @@ func testSend(ctx context.Context, c *mockCluster, from uint64, to []uint64, msg
105105
}
106106
}
107107

108+
// TestSplitSnapshotDataDoesNotMutateInput is a regression test for #3231.
109+
// Before the fix, splitSnapshotData did a shallow copy of the raft message
110+
// and re-sliced the shared Snapshot.Data on each iteration, shrinking the
111+
// original slice's capacity and eventually panicking with
112+
// "slice bounds out of range".
113+
func TestSplitSnapshotDataDoesNotMutateInput(t *testing.T) {
114+
ctx := context.Background()
115+
116+
// Build a MsgSnap whose Snapshot.Data clearly exceeds GRPCMaxMsgSize so
117+
// that the split loop runs multiple iterations (where the bug manifests).
118+
const dataSize = 3 * GRPCMaxMsgSize
119+
data := make([]byte, dataSize)
120+
for i := range data {
121+
data[i] = byte(i % (1 << 8))
122+
}
123+
m := raftpb.Message{
124+
Type: raftpb.MsgSnap,
125+
From: 1,
126+
To: 2,
127+
Snapshot: &raftpb.Snapshot{
128+
Data: data,
129+
Metadata: raftpb.SnapshotMetadata{
130+
Index: uint64(len(data)),
131+
},
132+
},
133+
}
134+
origData := m.Snapshot.Data
135+
origLen, origCap := len(origData), cap(origData)
136+
137+
msgs := splitSnapshotData(ctx, &m)
138+
require.Greater(t, len(msgs), 1, "data larger than GRPCMaxMsgSize must split into multiple chunks")
139+
140+
// Chunks must reassemble to the original data.
141+
var assembled []byte
142+
for _, msg := range msgs {
143+
assembled = append(assembled, msg.Message.Snapshot.Data...)
144+
}
145+
assert.Equal(t, data, assembled)
146+
147+
// The input message's Snapshot.Data must be untouched (regression guard).
148+
assert.Equal(t, origLen, len(m.Snapshot.Data))
149+
assert.Equal(t, origCap, cap(m.Snapshot.Data))
150+
assert.Equal(t, data, m.Snapshot.Data)
151+
}
152+
108153
func TestSend(t *testing.T) {
109154
ctx, cancel := context.WithCancel(context.Background())
110155
c := newCluster()

0 commit comments

Comments
 (0)