From 4b78ef86d8972f5c818b899c231b12abaa141c6e Mon Sep 17 00:00:00 2001 From: folbrich Date: Sat, 14 Feb 2026 10:35:29 +0100 Subject: [PATCH] Fix bugs, resource leaks, and test issues in cmd/desync - store.go: Fix wrong variable in S3 bucket lookup error message (printed nil IndexStore instead of the lookup string) - chunkserver.go, mount-index.go: Fix errors.Wrapf passing err as format arg instead of the store-file path - info.go: Fix missing closing quote in unsupported format error string - cat.go, inspectchunks.go: Close output file when writing to disk - chunkserver.go: Initialize loggingResponseWriter statusCode to 200 so implicit WriteHeader is logged correctly - config.go: Write config JSON to stdout instead of stderr in read mode - prune.go: Check error from fmt.Fscanln to prevent infinite loop on closed stdin - chunkserver_test.go, indexserver_test.go: Replace require.NoError (which calls t.FailNow) with t.Errorf in server goroutines - verifyindex_test.go: Remove no-op assertions that check for empty string --- cmd/desync/cat.go | 4 +++- cmd/desync/chunkserver.go | 4 ++-- cmd/desync/chunkserver_test.go | 5 +++-- cmd/desync/config.go | 2 +- cmd/desync/indexserver_test.go | 5 +++-- cmd/desync/info.go | 2 +- cmd/desync/inspectchunks.go | 4 +++- cmd/desync/mount-index.go | 2 +- cmd/desync/prune.go | 4 +++- cmd/desync/store.go | 2 +- cmd/desync/verifyindex_test.go | 2 -- 11 files changed, 21 insertions(+), 15 deletions(-) diff --git a/cmd/desync/cat.go b/cmd/desync/cat.go index 5d1b1ec..7206cb2 100644 --- a/cmd/desync/cat.go +++ b/cmd/desync/cat.go @@ -60,10 +60,12 @@ func runCat(ctx context.Context, opt catOptions, args []string) error { ) if len(args) == 2 { outFileName := args[1] - outFile, err = os.Create(outFileName) + f, err := os.Create(outFileName) if err != nil { return err } + defer f.Close() + outFile = f } else { outFile = stdout } diff --git a/cmd/desync/chunkserver.go b/cmd/desync/chunkserver.go index bbab6a4..c259a93 100644 --- a/cmd/desync/chunkserver.go +++ b/cmd/desync/chunkserver.go @@ -157,7 +157,7 @@ func runChunkServer(ctx context.Context, opt chunkServerOptions, args []string) // Wrapper for http.HandlerFunc to add logging for requests (and response codes) func withLog(h http.Handler, log *log.Logger) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - lrw := &loggingResponseWriter{ResponseWriter: w} + lrw := &loggingResponseWriter{ResponseWriter: w, statusCode: http.StatusOK} h.ServeHTTP(lrw, r) log.Printf("Client: %s, Request: %s %s, Response: %d", r.RemoteAddr, r.Method, r.RequestURI, lrw.statusCode) } @@ -178,7 +178,7 @@ func chunkServerStore(opt chunkServerOptions) (desync.Store, error) { } stores, cache, err = readStoreFile(opt.storeFile) if err != nil { - return nil, errors.Wrapf(err, "failed to read store-file '%s'", err) + return nil, errors.Wrapf(err, "failed to read store-file '%s'", opt.storeFile) } } diff --git a/cmd/desync/chunkserver_test.go b/cmd/desync/chunkserver_test.go index 51b49ce..90d2d09 100644 --- a/cmd/desync/chunkserver_test.go +++ b/cmd/desync/chunkserver_test.go @@ -173,8 +173,9 @@ func startChunkServer(t *testing.T, args ...string) (string, context.CancelFunc) cmd := newChunkServerCommand(ctx) cmd.SetArgs(append(args, "-l", addr)) go func() { - _, err = cmd.ExecuteC() - require.NoError(t, err) + if _, err := cmd.ExecuteC(); err != nil && ctx.Err() == nil { + t.Errorf("chunk server error: %v", err) + } }() // Wait a little for the server to start diff --git a/cmd/desync/config.go b/cmd/desync/config.go index e43c1d7..b571637 100644 --- a/cmd/desync/config.go +++ b/cmd/desync/config.go @@ -120,7 +120,7 @@ func runConfig(ctx context.Context, write bool) error { if err != nil { return err } - var w io.Writer = os.Stderr + var w io.Writer = os.Stdout if write { if err = os.MkdirAll(filepath.Dir(cfgFile), 0755); err != nil { return err diff --git a/cmd/desync/indexserver_test.go b/cmd/desync/indexserver_test.go index 120c80e..9092209 100644 --- a/cmd/desync/indexserver_test.go +++ b/cmd/desync/indexserver_test.go @@ -83,8 +83,9 @@ func startIndexServer(t *testing.T, args ...string) (string, context.CancelFunc) cmd := newIndexServerCommand(ctx) cmd.SetArgs(append(args, "-l", addr)) go func() { - _, err = cmd.ExecuteC() - require.NoError(t, err) + if _, err := cmd.ExecuteC(); err != nil && ctx.Err() == nil { + t.Errorf("index server error: %v", err) + } }() // Wait a little for the server to start diff --git a/cmd/desync/info.go b/cmd/desync/info.go index ea5298c..719d61c 100644 --- a/cmd/desync/info.go +++ b/cmd/desync/info.go @@ -242,7 +242,7 @@ func runInfo(ctx context.Context, opt infoOptions, args []string) error { fmt.Println("Chunk size avg:", results.ChunkSizeAvg) fmt.Println("Chunk size max:", results.ChunkSizeMax) default: - return fmt.Errorf("unsupported output format '%s", opt.printFormat) + return fmt.Errorf("unsupported output format %q", opt.printFormat) } return nil } diff --git a/cmd/desync/inspectchunks.go b/cmd/desync/inspectchunks.go index 21da5e5..92da0d7 100644 --- a/cmd/desync/inspectchunks.go +++ b/cmd/desync/inspectchunks.go @@ -48,10 +48,12 @@ func runInspectChunks(ctx context.Context, opt inspectChunksOptions, args []stri ) if len(args) == 2 { outFileName := args[1] - outFile, err = os.Create(outFileName) + f, err := os.Create(outFileName) if err != nil { return err } + defer f.Close() + outFile = f } else { outFile = stdout } diff --git a/cmd/desync/mount-index.go b/cmd/desync/mount-index.go index f5d89ff..ea597a8 100644 --- a/cmd/desync/mount-index.go +++ b/cmd/desync/mount-index.go @@ -157,7 +157,7 @@ func mountIndexStore(opt mountIndexOptions) (desync.Store, error) { } stores, cache, err = readStoreFile(opt.storeFile) if err != nil { - return nil, errors.Wrapf(err, "failed to read store-file '%s'", err) + return nil, errors.Wrapf(err, "failed to read store-file '%s'", opt.storeFile) } } diff --git a/cmd/desync/prune.go b/cmd/desync/prune.go index 0778314..57aa2e3 100644 --- a/cmd/desync/prune.go +++ b/cmd/desync/prune.go @@ -85,7 +85,9 @@ func runPrune(ctx context.Context, opt pruneOptions, args []string) error { for { var a string fmt.Printf("[y/N]: ") - fmt.Fscanln(os.Stdin, &a) + if _, err := fmt.Fscanln(os.Stdin, &a); err != nil { + return err + } switch a { case "y", "Y": break ask diff --git a/cmd/desync/store.go b/cmd/desync/store.go index 5cbbd7d..cb5b2e6 100644 --- a/cmd/desync/store.go +++ b/cmd/desync/store.go @@ -257,7 +257,7 @@ func indexStoreFromLocation(location string, cmdOpt cmdStoreOptions) (desync.Ind lookup = minio.BucketLookupPath case "", "auto": default: - return nil, "", fmt.Errorf("unknown S3 bucket lookup type: %q", s) + return nil, "", fmt.Errorf("unknown S3 bucket lookup type: %q", ls) } s, err = desync.NewS3IndexStore(&p, s3Creds, region, opt, lookup) if err != nil { diff --git a/cmd/desync/verifyindex_test.go b/cmd/desync/verifyindex_test.go index 29d35b9..2fed403 100644 --- a/cmd/desync/verifyindex_test.go +++ b/cmd/desync/verifyindex_test.go @@ -15,7 +15,6 @@ func TestVerifyIndexCommand(t *testing.T) { stderr = b _, err := verifyIndex.ExecuteC() require.NoError(t, err) - require.Contains(t, b.String(), "") // Do the same for blob2 verifyIndex = newVerifyIndexCommand(context.Background()) @@ -24,7 +23,6 @@ func TestVerifyIndexCommand(t *testing.T) { stderr = b _, err = verifyIndex.ExecuteC() require.NoError(t, err) - require.Contains(t, b.String(), "") // Run again against the wrong blob verifyIndex = newVerifyIndexCommand(context.Background())