From 2fc104ee07ff617931db783be38344f94079926c Mon Sep 17 00:00:00 2001 From: xh123637 <3324733895@qq.com> Date: Thu, 12 Nov 2026 16:15:29 +0800 Subject: [PATCH 1/2] container/cp: don't print success message when copy fails The non-quiet copy paths printed the "Successfully copied ..." summary unconditionally, even when the copy failed. Copying a directory onto an existing regular file therefore printed "Successfully copied 0B" before reporting "cannot copy directory". Only print the summary when the copy succeeded, in both the from-container and to-container directions. Fixes: https://github.com/docker/cli/issues/7514 Signed-off-by: xh123637 <3324733895@qq.com> --- cli/command/container/cp.go | 20 ++++++----- cli/command/container/cp_test.go | 62 ++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index fbce9df646e5..40b1a4d3b01f 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -339,11 +339,13 @@ func copyFromContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cp cancel() <-done restore() - reportedSize := copiedSize - if !cpRes.Stat.Mode.IsDir() { - reportedSize = cpRes.Stat.Size + if res == nil { + reportedSize := copiedSize + if !cpRes.Stat.Mode.IsDir() { + reportedSize = cpRes.Stat.Size + } + _, _ = fmt.Fprint(dockerCLI.Err(), copySummary(reportedSize, copiedSize, dstPath)) } - _, _ = fmt.Fprint(dockerCLI.Err(), copySummary(reportedSize, copiedSize, dstPath)) return res } @@ -474,11 +476,13 @@ func copyToContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cpCo cancel() <-done restore() - reportedSize := copiedSize - if sizeErr == nil { - reportedSize = contentSize + if err == nil { + reportedSize := copiedSize + if sizeErr == nil { + reportedSize = contentSize + } + _, _ = fmt.Fprint(dockerCLI.Err(), copySummary(reportedSize, copiedSize, copyConfig.container+":"+dstInfo.Path)) } - _, _ = fmt.Fprint(dockerCLI.Err(), copySummary(reportedSize, copiedSize, copyConfig.container+":"+dstInfo.Path)) return err } diff --git a/cli/command/container/cp_test.go b/cli/command/container/cp_test.go index e193ebbf411b..d2ce73b0d30d 100644 --- a/cli/command/container/cp_test.go +++ b/cli/command/container/cp_test.go @@ -2,6 +2,7 @@ package container import ( "context" + "errors" "io" "os" "runtime" @@ -446,3 +447,64 @@ func TestCopyToContainerStdinReportsTransferredSize(t *testing.T) { // "(transferred ...)" should not appear. assert.Check(t, !strings.Contains(errOut, "(transferred")) } + +func TestCopyFromContainerDoesNotReportSuccessOnFailure(t *testing.T) { + // Copying a directory onto an existing regular file fails while + // extracting ("cannot copy directory"). The failure must not be + // preceded by a "Successfully copied" message. + srcDir := fs.NewDir(t, "cp-test-fromdir-fail", + fs.WithFile("file1", "hello")) + destFile := fs.NewFile(t, "cp-test-fromdir-fail-dest", fs.WithContent("original\n")) + + fakeCli := test.NewFakeCli(&fakeClient{ + containerCopyFromFunc: func(ctr, srcPath string) (client.CopyFromContainerResult, error) { + readCloser, err := archive.Tar(srcDir.Path(), compression.None) + return client.CopyFromContainerResult{ + Content: readCloser, + Stat: container.PathStat{ + Name: "mydir", + Mode: os.ModeDir | 0o755, + }, + }, err + }, + }) + err := runCopy(context.TODO(), fakeCli, copyOptions{ + source: "container:/mydir", + destination: destFile.Path(), + }) + assert.ErrorContains(t, err, "cannot copy directory") + assert.Check(t, !strings.Contains(fakeCli.ErrBuffer().String(), "Successfully copied"), + "unexpected success message in output: %q", fakeCli.ErrBuffer().String()) + + content, err := os.ReadFile(destFile.Path()) + assert.NilError(t, err) + assert.Check(t, is.Equal("original\n", string(content))) +} + +func TestCopyToContainerDoesNotReportSuccessOnFailure(t *testing.T) { + // A failed upload must not be followed by a "Successfully copied" + // message. + srcFile := fs.NewFile(t, "cp-test-to-fail", fs.WithContent("hello")) + + fakeCli := test.NewFakeCli(&fakeClient{ + containerStatPathFunc: func(containerID, path string) (client.ContainerStatPathResult, error) { + return client.ContainerStatPathResult{ + Stat: container.PathStat{ + Name: "tmp", + Mode: os.ModeDir | 0o755, + }, + }, nil + }, + containerCopyToFunc: func(containerID string, options client.CopyToContainerOptions) (client.CopyToContainerResult, error) { + _, _ = io.Copy(io.Discard, options.Content) + return client.CopyToContainerResult{}, errors.New("copy to container failed") + }, + }) + err := runCopy(context.TODO(), fakeCli, copyOptions{ + source: srcFile.Path(), + destination: "container:/tmp", + }) + assert.ErrorContains(t, err, "copy to container failed") + assert.Check(t, !strings.Contains(fakeCli.ErrBuffer().String(), "Successfully copied"), + "unexpected success message in output: %q", fakeCli.ErrBuffer().String()) +} From 317a13c5253471a708221e79d6870096da25e49e Mon Sep 17 00:00:00 2001 From: xh123637 <3324733895@qq.com> Date: Thu, 12 Nov 2026 20:23:05 +0800 Subject: [PATCH 2/2] container/cp: rename res to err in copyFromContainer The variable only holds the error returned by archive.CopyTo, so name it err to make that clear instead of suggesting some other "result" return type. Signed-off-by: xh123637 <3324733895@qq.com> --- cli/command/container/cp.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index 40b1a4d3b01f..aa46ce117637 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -335,11 +335,11 @@ func copyFromContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cp } restore, done := copyProgress(ctx, dockerCLI.Err(), copyFromContainerHeader, &copiedSize) - res := archive.CopyTo(preArchive, srcInfo, dstPath) + err = archive.CopyTo(preArchive, srcInfo, dstPath) cancel() <-done restore() - if res == nil { + if err == nil { reportedSize := copiedSize if !cpRes.Stat.Mode.IsDir() { reportedSize = cpRes.Stat.Size @@ -347,7 +347,7 @@ func copyFromContainer(ctx context.Context, dockerCLI command.Cli, copyConfig cp _, _ = fmt.Fprint(dockerCLI.Err(), copySummary(reportedSize, copiedSize, dstPath)) } - return res + return err } // In order to get the copy behavior right, we need to know information