Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
11 changes: 4 additions & 7 deletions http/respChain.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,9 @@ func (r *ResponseChain) HeadersBytes() []byte {

// HeadersString returns the current response headers as string in the chain.
//
// The returned string is valid only until Close() is called.
// This is a zero-copy operation for performance.
// The returned string is a copy and remains valid even after Close() is called.
func (r *ResponseChain) HeadersString() string {
return conversion.String(r.headers.Bytes())
return r.headers.String()
}

// Body returns the current response body buffer in the chain.
Expand All @@ -282,10 +281,9 @@ func (r *ResponseChain) BodyBytes() []byte {

// BodyString returns the current response body as string in the chain.
//
// The returned string is valid only until Close() is called.
// This is a zero-copy operation for performance.
// The returned string is a copy and remains valid even after Close() is called.
func (r *ResponseChain) BodyString() string {
return conversion.String(r.body.Bytes())
return r.body.String()
}

// FullResponse returns a new buffer containing headers+body.
Expand Down Expand Up @@ -319,7 +317,6 @@ func (r *ResponseChain) FullResponseBytes() []byte {
// FullResponseString returns the current response as string in the chain.
//
// The returned string is a copy and remains valid even after Close() is called.
// This is a zero-copy operation from the byte slice.
func (r *ResponseChain) FullResponseString() string {
return conversion.String(r.FullResponseBytes())
Comment on lines 317 to 321
Copy link
Member Author

Choose a reason for hiding this comment

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

FullResponseString() is already safe because FullResponseBytes() returns a new slice, so I will leave it as is to maintain the zero-copy optimization on that specific method (avoiding a second copy).

}
Expand Down
45 changes: 45 additions & 0 deletions http/respChain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1182,3 +1182,48 @@ func TestLimitedBuffer_Pool(t *testing.T) {
require.Equal(t, len(data), buf.Len())
require.Equal(t, data, buf.Bytes())
}

func TestResponseChain_StringSafety(t *testing.T) {
bodyContent := "Original Body Content That Should Be Preserved Yeah Okay LOL"
headerKey := "X-Safety-Test"
headerValue := "Original Header Value"

resp := &http.Response{
StatusCode: 200,
Body: io.NopCloser(strings.NewReader(bodyContent)),
Header: http.Header{headerKey: []string{headerValue}},
Proto: "HTTP/1.1",
ProtoMajor: 1,
ProtoMinor: 1,
}

rc := NewResponseChain(resp, -1)
err := rc.Fill()
require.NoError(t, err)

bodyStr := rc.BodyString()
headersStr := rc.HeadersString()

assert.Equal(t, bodyContent, bodyStr)
assert.Contains(t, headersStr, headerValue)

rc.Close()

// Now attempt to pollute the pool and overwrite the memory.
// We get a bunch of buffers and write garbage to them.
var buffers []*bytes.Buffer
for i := 0; i < 100; i++ {
buf := getBuffer()
buffers = append(buffers, buf)

buf.Reset()
buf.WriteString("ALERTA_GARBAGE_DATA_OVERWRITING_MEMORY_ALERTA_GARBAGE_DATA_OVERWRITING_MEMORY")
}

for _, buf := range buffers {
putBuffer(buf)
}

assert.Equal(t, bodyContent, bodyStr, "BodyString() content changed after buffer reuse - unsafe memory sharing detected")
assert.Contains(t, headersStr, headerValue, "HeadersString() content changed after buffer reuse - unsafe memory sharing detected")
}
Loading