Skip to content

Commit 5cb71ec

Browse files
authored
Merge pull request kubernetes#123620 from benluddy/json-frame-reader-underlying-array-reuse
Remove shared ref to underlying array of JSONFrameReader's Read arg.
2 parents 3d24b96 + 10f7a16 commit 5cb71ec

File tree

2 files changed

+27
-6
lines changed

2 files changed

+27
-6
lines changed

staging/src/k8s.io/apimachinery/pkg/util/framer/framer.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@ func (r *jsonFrameReader) Read(data []byte) (int, error) {
147147

148148
// RawMessage#Unmarshal appends to data - we reset the slice down to 0 and will either see
149149
// data written to data, or be larger than data and a different array.
150-
n := len(data)
151150
m := json.RawMessage(data[:0])
152151
if err := r.decoder.Decode(&m); err != nil {
153152
return 0, err
@@ -156,12 +155,19 @@ func (r *jsonFrameReader) Read(data []byte) (int, error) {
156155
// If capacity of data is less than length of the message, decoder will allocate a new slice
157156
// and set m to it, which means we need to copy the partial result back into data and preserve
158157
// the remaining result for subsequent reads.
159-
if len(m) > n {
160-
//nolint:staticcheck // SA4006,SA4010 underlying array of data is modified here.
161-
data = append(data[0:0], m[:n]...)
162-
r.remaining = m[n:]
163-
return n, io.ErrShortBuffer
158+
if len(m) > cap(data) {
159+
copy(data, m)
160+
r.remaining = m[len(data):]
161+
return len(data), io.ErrShortBuffer
164162
}
163+
164+
if len(m) > len(data) {
165+
// The bytes beyond len(data) were stored in data's underlying array, which we do
166+
// not own after this function returns.
167+
r.remaining = append([]byte(nil), m[len(data):]...)
168+
return len(data), io.ErrShortBuffer
169+
}
170+
165171
return len(m), nil
166172
}
167173

staging/src/k8s.io/apimachinery/pkg/util/framer/framer_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package framer
1818

1919
import (
2020
"bytes"
21+
"errors"
2122
"io"
2223
"testing"
2324
)
@@ -173,3 +174,17 @@ func TestJSONFrameReaderShortBuffer(t *testing.T) {
173174
t.Fatalf("unexpected: %v %d %q", err, n, buf)
174175
}
175176
}
177+
178+
func TestJSONFrameReaderShortBufferNoUnderlyingArrayReuse(t *testing.T) {
179+
b := bytes.NewBufferString("{}")
180+
r := NewJSONFramedReader(io.NopCloser(b))
181+
buf := make([]byte, 1, 2) // cap(buf) > len(buf) && cap(buf) <= len("{}")
182+
183+
if n, err := r.Read(buf); !errors.Is(err, io.ErrShortBuffer) || n != 1 || string(buf[:n]) != "{" {
184+
t.Fatalf("unexpected: %v %d %q", err, n, buf)
185+
}
186+
buf = append(buf, make([]byte, 1)...) // stomps the second byte of the backing array
187+
if n, err := r.Read(buf[1:]); err != nil || n != 1 || string(buf[1:1+n]) != "}" {
188+
t.Fatalf("unexpected: %v %d %q", err, n, buf)
189+
}
190+
}

0 commit comments

Comments
 (0)