diff --git a/cmd/storage-rest-common.go b/cmd/storage-rest-common.go index d8a28ae29..17bcc5f69 100644 --- a/cmd/storage-rest-common.go +++ b/cmd/storage-rest-common.go @@ -83,3 +83,13 @@ type nsScannerResp struct { Update *dataUsageEntry `msg:"u"` Final *dataUsageCache `msg:"f"` } + +// Recycle resets the response, so that recycled responses cannot +// leak stale scanner data into a subsequent scan. +func (r *nsScannerResp) Recycle() { + if r == nil { + return + } + r.Update = nil + r.Final = nil +} diff --git a/cmd/storage-rest-server.go b/cmd/storage-rest-server.go index f20e4e99f..17d274bca 100644 --- a/cmd/storage-rest-server.go +++ b/cmd/storage-rest-server.go @@ -290,6 +290,9 @@ func (s *storageRESTServer) NSScannerHandler(ctx context.Context, params *nsScan for update := range updates { resp := storageNSScannerRPC.NewResponse() resp.Update = &update + // Pooled responses may carry fields from a previous + // scan, always clear the fields not being set. + resp.Final = nil out <- resp } }() @@ -301,6 +304,7 @@ func (s *storageRESTServer) NSScannerHandler(ctx context.Context, params *nsScan // Send final response. resp := storageNSScannerRPC.NewResponse() resp.Final = &ui + resp.Update = nil out <- resp return nil } diff --git a/cmd/storage-rest-server_test.go b/cmd/storage-rest-server_test.go new file mode 100644 index 000000000..354348882 --- /dev/null +++ b/cmd/storage-rest-server_test.go @@ -0,0 +1,36 @@ +// Copyright (c) 2015-2023 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import "testing" + +// TestNSScannerRespPooling verifies that recycled NSScanner stream +// responses cannot leak stale fields from a previous scan into new +// responses, which would otherwise produce incorrect bucket usage +// data. +func TestNSScannerRespPooling(t *testing.T) { + dirty := storageNSScannerRPC.NewResponse() + dirty.Update = &dataUsageEntry{} + dirty.Final = &dataUsageCache{} + storageNSScannerRPC.PutResponse(dirty) + + resp := storageNSScannerRPC.NewResponse() + if resp.Update != nil || resp.Final != nil { + t.Fatalf("pooled nsScannerResp leaked stale fields: Update %v, Final %v", resp.Update, resp.Final) + } +} diff --git a/internal/grid/handlers.go b/internal/grid/handlers.go index 1d20fa9b9..b0ca9e324 100644 --- a/internal/grid/handlers.go +++ b/internal/grid/handlers.go @@ -698,6 +698,11 @@ func (h *StreamTypeHandler[Payload, Req, Resp]) PutRequest(r Req) { // These should be returned by the caller. func (h *StreamTypeHandler[Payload, Req, Resp]) PutResponse(r Resp) { if r != h.nilResp { + if rc, ok := any(r).(Recycler); ok { + // Recycled stream responses must not leak fields + // from a previous use into the next response. + rc.Recycle() + } //nolint:staticcheck // SA6002 IT IS A GENERIC VALUE! (and always a pointer) h.respPool.Put(r) }