diff --git a/sql/atxs/atxs.go b/sql/atxs/atxs.go index 629f0b0d79..687aafb682 100644 --- a/sql/atxs/atxs.go +++ b/sql/atxs/atxs.go @@ -301,10 +301,6 @@ func GetBlobSizes(db sql.Executor, ids [][]byte) (sizes []int, err error) { } // LoadBlob loads ATX as an encoded blob, ready to be sent over the wire. -// -// SAFETY: The contents of the returned blob MUST NOT be modified. -// They might point to the inner sql cache and modifying them would -// corrupt the cache. func LoadBlob(ctx context.Context, db sql.Executor, id []byte, blob *sql.Blob) (types.AtxVersion, error) { if sql.IsCached(db) { type cachedBlob struct { @@ -325,8 +321,8 @@ func LoadBlob(ctx context.Context, db sql.Executor, id []byte, blob *sql.Blob) ( if err != nil { return 0, err } - // Here we return the cached slice, hence the safety warning. - blob.Bytes = cached.buf + blob.Resize(len(cached.buf)) + copy(blob.Bytes, cached.buf) return cached.version, nil } diff --git a/sql/atxs/atxs_test.go b/sql/atxs/atxs_test.go index cc13ba4d9e..7bfa8eb7e3 100644 --- a/sql/atxs/atxs_test.go +++ b/sql/atxs/atxs_test.go @@ -647,6 +647,26 @@ func TestGetBlobCached_CacheEntriesAreDistinct(t *testing.T) { require.Equal(t, atx.AtxBlob.Blob, blob.Bytes) } +// Test that the cached blob is not shared with the caller +// but copied into the provided blob. +func TestGetBlobCached_OverwriteSafety(t *testing.T) { + db := sql.InMemory(sql.WithQueryCache(true)) + atx := types.ActivationTx{} + atx.SetID(types.RandomATXID()) + atx.AtxBlob.Blob = []byte("original blob") + require.NoError(t, atxs.Add(db, &atx)) + require.Equal(t, 2, db.QueryCount()) // insert atx + blob + + var b sql.Blob // we will reuse the blob between queries + _, err := atxs.LoadBlob(context.Background(), db, atx.ID().Bytes(), &b) + require.NoError(t, err) + require.Equal(t, atx.AtxBlob.Blob, b.Bytes) + b.Bytes[0] = 'X' // modify the blob + _, err = atxs.LoadBlob(context.Background(), db, atx.ID().Bytes(), &b) + require.NoError(t, err) + require.Equal(t, atx.AtxBlob.Blob, b.Bytes) +} + func TestCachedBlobEviction(t *testing.T) { db := sql.InMemory( sql.WithQueryCache(true), diff --git a/sql/database.go b/sql/database.go index 799a19293f..907f8d1dde 100644 --- a/sql/database.go +++ b/sql/database.go @@ -505,10 +505,10 @@ type Blob struct { Bytes []byte } -// resize the underlying byte slice to the specified size. +// Resize the underlying byte slice to the specified size. // The returned slice has length equal n, but it might have a larger capacity. // Warning: it is not guaranteed to keep the old data. -func (b *Blob) resize(n int) { +func (b *Blob) Resize(n int) { if cap(b.Bytes) < n { b.Bytes = make([]byte, n) } @@ -517,10 +517,10 @@ func (b *Blob) resize(n int) { func (b *Blob) FromColumn(stmt *Statement, col int) { if l := stmt.ColumnLen(col); l != 0 { - b.resize(l) + b.Resize(l) stmt.ColumnBytes(col, b.Bytes) } else { - b.resize(0) + b.Resize(0) } }