Decoded bytes use the same underlying array
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 1.1k
- Forks
- 232
- PR merge metrics
- No merged PRs in 30d
Description
When decoding data that contains bytes fields, the returned byte slices reference the same underlying array as the input. This can cause unexpected behavior when manipulating those byte slices. Specifically appending to these slices is not a safe operation, because they can change other byte slices.
Click to expand failing test case
Notice how appending to the slice `foo` had an effect on `bar` and `binary`. All three slices use the same underlying array.
```go
package avro
import (
"bytes"
"testing"
"github.com/linkedin/goavro/v2"
)
func TestBytesOverwrite(t *testing.T) {
schema := `
{
"name":"example",
"type":"record",
"fields":[
{"name":"foo","type":"bytes"},
{"name":"bar","type":"bytes"}
]
}`
codec, err := goavro.NewCodec(schema)
if err != nil {
t.Fatal(err)
}
// binary data contains {foo:[1,2],bar:[3,4]}
binary := []byte{4, 1, 2, 4, 3, 4}
gotNative, _, err := codec.NativeFromBinary(binary)
if err != nil {
t.Fatal(err)
}
got := gotNative.(map[string]interface{})
foo := got["foo"].([]byte)
bar := got["bar"].([]byte)
if want := []byte{1, 2}; !bytes.Equal(foo, want) {
t.Fatalf("expected foo to be %v, actually got %v", want, foo)
}
if want := []byte{3, 4}; !bytes.Equal(bar, want) {
t.Fatalf("expected bar to be %v, actually got %v", want, bar)
}
// because slices use the same underlying array we can overwrite the others
// by appending to foo
foo = append(foo, 0, 0)
if want := []byte{1, 2, 0, 0}; !bytes.Equal(foo, want) {
t.Fatalf("expected foo to be %v, actually got %v", want, foo)
}
if want := []byte{3, 4}; !bytes.Equal(bar, want) {
// this assertion fails, bar was changed to [0, 4]
t.Errorf("expected bar to be %v, actually got %v", want, bar)
}
if want := []byte{4, 1, 2, 4, 3, 4}; !bytes.Equal(binary, want) {
// this assertion fails, binary was changed to [4, 1, 2, 0, 0, 4]
t.Errorf("expected binary to be %v, actually got %v", want, binary)
}
}
```
This could probably be fixed by allocating a new slice before returning `buf[:size]` in [this line](https://github.com/linkedin/goavro/blob/8eb9f0e2d756cea165f593f80c6780f4b0b4dbb6/bytes.go#L42).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with bytes.go at line 42 and the embedded TestBytesOverwrite case. Trace how NativeFromBinary returns byte slices, then use the test case to verify that appending to foo does not alter bar or the original binary input. Done when all assertions pass with independent decoded slices.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- data
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100