client: CopyToContainer(), CopyFromContainer(): remove status-code handling

This was added in 93c3e6c91ec5eb4202b86b44b011d06f5e048dab, at which time only
some basic handling of non-succesful status codes was present;
https://github.com/moby/moby/blob/93c3e6c91ec5eb4202b86b44b011d06f5e048dab/api/client/utils.go#L112-L121

Given that since 38e6d474affe08230043fa36fa02f623c2ab2661 non-successful status-
codes are already handled, and a 204 ("no content") status should not be an error,
this special case should no longer be needed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

client/container_copy.go

@@ -50,11 +50,6 @@ func (cli *Client) CopyToContainer(ctx context.Context, containerID, dstPath str
 		return err
 	}
 
-	// TODO this code converts non-error status-codes (e.g., "204 No Content") into an error; verify if this is the desired behavior
-	if response.statusCode != http.StatusOK {
-		return fmt.Errorf("unexpected status code from daemon: %d", response.statusCode)
-	}
-
 	return nil
 }
 
@@ -70,11 +65,6 @@ func (cli *Client) CopyFromContainer(ctx context.Context, containerID, srcPath s
 		return nil, types.ContainerPathStat{}, err
 	}
 
-	// TODO this code converts non-error status-codes (e.g., "204 No Content") into an error; verify if this is the desired behavior
-	if response.statusCode != http.StatusOK {
-		return nil, types.ContainerPathStat{}, fmt.Errorf("unexpected status code from daemon: %d", response.statusCode)
-	}
-
 	// In order to get the copy behavior right, we need to know information
 	// about both the source and the destination. The response headers include
 	// stat info about the source that we can use in deciding exactly how to

client/container_copy_test.go

@@ -115,14 +115,15 @@ func TestCopyToContainerNotFoundError(t *testing.T) {
 	}
 }
 
-// TODO TestCopyToContainerNotStatusOKError expects a non-error status-code ("204 No Content") to produce an error; verify if this is the desired behavior
-func TestCopyToContainerNotStatusOKError(t *testing.T) {
+// TestCopyToContainerEmptyResponse verifies that no error is returned when a
+// "204 No Content" is returned by the API.
+func TestCopyToContainerEmptyResponse(t *testing.T) {
 	client := &Client{
 		client: newMockClient(errorMock(http.StatusNoContent, "No content")),
 	}
 	err := client.CopyToContainer(context.Background(), "container_id", "path/to/file", bytes.NewReader([]byte("")), types.CopyToContainerOptions{})
-	if err == nil || err.Error() != "unexpected status code from daemon: 204" {
-		t.Fatalf("expected an unexpected status code error, got %v", err)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
 	}
 }
 
@@ -192,14 +193,30 @@ func TestCopyFromContainerNotFoundError(t *testing.T) {
 	}
 }
 
-// TODO TestCopyFromContainerNotStatusOKError expects a non-error status-code ("204 No Content") to produce an error; verify if this is the desired behavior
-func TestCopyFromContainerNotStatusOKError(t *testing.T) {
+// TestCopyFromContainerEmptyResponse verifies that no error is returned when a
+// "204 No Content" is returned by the API.
+func TestCopyFromContainerEmptyResponse(t *testing.T) {
 	client := &Client{
-		client: newMockClient(errorMock(http.StatusNoContent, "No content")),
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			content, err := json.Marshal(types.ContainerPathStat{
+				Name: "path/to/file",
+				Mode: 0700,
+			})
+			if err != nil {
+				return nil, err
+			}
+			base64PathStat := base64.StdEncoding.EncodeToString(content)
+			return &http.Response{
+				StatusCode: http.StatusNoContent,
+				Header: http.Header{
+					"X-Docker-Container-Path-Stat": []string{base64PathStat},
+				},
+			}, nil
+		}),
 	}
 	_, _, err := client.CopyFromContainer(context.Background(), "container_id", "path/to/file")
-	if err == nil || err.Error() != "unexpected status code from daemon: 204" {
-		t.Fatalf("expected an unexpected status code error, got %v", err)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
 	}
 }
 

integration/container/copy_test.go

@@ -2,15 +2,18 @@ package container // import "github.com/docker/docker/integration/container"
 
 import (
 	"archive/tar"
+	"bytes"
 	"context"
 	"encoding/json"
 	"io"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/integration/internal/container"
+	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/testutil/fakecontext"
 	"gotest.tools/v3/assert"
@@ -59,6 +62,47 @@ func TestCopyToContainerPathDoesNotExist(t *testing.T) {
 	assert.ErrorContains(t, err, "Could not find the file /dne in container "+cid)
 }
 
+func TestCopyEmptyFile(t *testing.T) {
+	defer setupTest(t)()
+
+	tmpDir := t.TempDir()
+	srcPath := filepath.Join(tmpDir, "empty-file.txt")
+	err := os.WriteFile(srcPath, []byte(""), 0400)
+	assert.NilError(t, err)
+
+	// TODO(thaJeztah) Add utilities to the client to make steps below less complicated.
+	// Code below is taken from copyToContainer() in docker/cli.
+	srcInfo, err := archive.CopyInfoSourcePath(srcPath, false)
+	assert.NilError(t, err)
+
+	srcArchive, err := archive.TarResource(srcInfo)
+	assert.NilError(t, err)
+	defer srcArchive.Close()
+
+	ctrPath := "/empty-file.txt"
+	dstInfo := archive.CopyInfo{Path: ctrPath}
+	dstDir, preparedArchive, err := archive.PrepareArchiveCopy(srcArchive, srcInfo, dstInfo)
+	assert.NilError(t, err)
+	defer preparedArchive.Close()
+
+	ctx := context.Background()
+	apiclient := testEnv.APIClient()
+	cid := container.Create(ctx, t, apiclient)
+
+	// empty content
+	err = apiclient.CopyToContainer(ctx, cid, dstDir, bytes.NewReader([]byte("")), types.CopyToContainerOptions{})
+	assert.NilError(t, err)
+
+	// tar with empty file
+	err = apiclient.CopyToContainer(ctx, cid, dstDir, preparedArchive, types.CopyToContainerOptions{})
+	assert.NilError(t, err)
+
+	// copy from empty file
+	rdr, _, err := apiclient.CopyFromContainer(ctx, cid, dstDir)
+	assert.NilError(t, err)
+	defer rdr.Close()
+}
+
 func TestCopyToContainerPathIsNotDir(t *testing.T) {
 	defer setupTest(t)()