diff --git a/cmd/update.go b/cmd/update.go index 8232832c9..69847d719 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -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 } diff --git a/cmd/update_test.go b/cmd/update_test.go index e1af9e38c..e94e049ea 100644 --- a/cmd/update_test.go +++ b/cmd/update_test.go @@ -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