mirror of
https://github.com/pgsty/minio.git
synced 2026-08-09 07:43:29 +03:00
fix: return owned update download buffers
Replace bytebufferpool-backed return values with bytes.Buffer storage so the downloaded and compressed slices remain valid after downloadBinary returns. Close the zstd encoder on copy failure and propagate final close errors. Add round-trip and pool-reuse regression coverage for both returned buffers. Co-authored-by: ChatGPT <noreply@openai.com> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
+8
-13
@@ -19,6 +19,7 @@ package cmd
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"crypto"
|
||||
"crypto/tls"
|
||||
"encoding/hex"
|
||||
@@ -42,7 +43,6 @@ import (
|
||||
xnet "github.com/minio/pkg/v3/net"
|
||||
"github.com/minio/selfupdate"
|
||||
gopsutilcpu "github.com/shirou/gopsutil/v3/cpu"
|
||||
"github.com/valyala/bytebufferpool"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -532,26 +532,21 @@ func downloadBinary(u *url.URL, mode string) (binCompressed []byte, bin []byte,
|
||||
}
|
||||
defer xhttp.DrainBody(reader)
|
||||
|
||||
b := bytebufferpool.Get()
|
||||
bc := bytebufferpool.Get()
|
||||
defer func() {
|
||||
b.Reset()
|
||||
bc.Reset()
|
||||
var b, bc bytes.Buffer
|
||||
|
||||
bytebufferpool.Put(b)
|
||||
bytebufferpool.Put(bc)
|
||||
}()
|
||||
|
||||
w, err := zstd.NewWriter(bc)
|
||||
w, err := zstd.NewWriter(&bc)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
if _, err = io.Copy(w, io.TeeReader(reader, b)); err != nil {
|
||||
if _, err = io.Copy(w, io.TeeReader(reader, &b)); err != nil {
|
||||
_ = w.Close()
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
w.Close()
|
||||
if err = w.Close(); err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
return bc.Bytes(), b.Bytes(), nil
|
||||
}
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
package cmd
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/hex"
|
||||
"fmt"
|
||||
"net/http"
|
||||
@@ -28,8 +29,73 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/klauspost/compress/zstd"
|
||||
"github.com/valyala/bytebufferpool"
|
||||
)
|
||||
|
||||
func TestDownloadBinaryReturnsOwnedBuffers(t *testing.T) {
|
||||
previousMaxProcs := runtime.GOMAXPROCS(1)
|
||||
t.Cleanup(func() {
|
||||
runtime.GOMAXPROCS(previousMaxProcs)
|
||||
})
|
||||
|
||||
payload := []byte("minio update payload")
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
_, _ = w.Write(payload)
|
||||
}))
|
||||
t.Cleanup(server.Close)
|
||||
|
||||
u, err := url.Parse(server.URL)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
compressed, downloaded, err := downloadBinary(u, "server")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !bytes.Equal(downloaded, payload) {
|
||||
t.Fatalf("downloaded binary is %q, want %q", downloaded, payload)
|
||||
}
|
||||
decoder, err := zstd.NewReader(nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(decoder.Close)
|
||||
decompressed, err := decoder.DecodeAll(compressed, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !bytes.Equal(decompressed, payload) {
|
||||
t.Fatalf("decompressed binary is %q, want %q", decompressed, payload)
|
||||
}
|
||||
wantCompressed := bytes.Clone(compressed)
|
||||
wantDownloaded := bytes.Clone(downloaded)
|
||||
|
||||
// Reuse buffers returned to bytebufferpool. downloadBinary's results must
|
||||
// remain valid after the function returns, regardless of later pool users.
|
||||
pooled := make([]*bytebufferpool.ByteBuffer, 8)
|
||||
for i := range pooled {
|
||||
pooled[i] = bytebufferpool.Get()
|
||||
if cap(pooled[i].B) > 0 {
|
||||
pooled[i].B = pooled[i].B[:cap(pooled[i].B)]
|
||||
for j := range pooled[i].B {
|
||||
pooled[i].B[j] = 0xa5
|
||||
}
|
||||
}
|
||||
}
|
||||
for _, b := range pooled {
|
||||
bytebufferpool.Put(b)
|
||||
}
|
||||
|
||||
if !bytes.Equal(compressed, wantCompressed) {
|
||||
t.Fatal("compressed download aliases a buffer returned to bytebufferpool")
|
||||
}
|
||||
if !bytes.Equal(downloaded, wantDownloaded) {
|
||||
t.Fatal("downloaded binary aliases a buffer returned to bytebufferpool")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMinioVersionToReleaseTime(t *testing.T) {
|
||||
testCases := []struct {
|
||||
version string
|
||||
|
||||
Reference in New Issue
Block a user