From c5aedacb4faf593f6cb03d6bc6c718f2ccc133fe Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 30 Oct 2025 09:55:45 +0100 Subject: [PATCH] client: Client.ContainerExport: close reader on context cancellation Use a cancelReadCloser to automatically close the reader when the context is cancelled. Consumers are still recommended to manually close the reader, but the cancelReadCloser makes the Close idempotent. Signed-off-by: Sebastiaan van Stijn --- client/container_export.go | 21 ++++--------------- client/container_export_test.go | 13 ++++++------ integration-cli/docker_api_containers_test.go | 6 +++--- integration/container/export_test.go | 3 ++- integration/container/overlayfs_linux_test.go | 8 +++---- integration/plugin/authz/authz_plugin_test.go | 8 +++---- .../moby/moby/client/container_export.go | 21 ++++--------------- 7 files changed, 27 insertions(+), 53 deletions(-) diff --git a/client/container_export.go b/client/container_export.go index d2929ee4b6..2d33efb7d8 100644 --- a/client/container_export.go +++ b/client/container_export.go @@ -19,6 +19,8 @@ type ContainerExportResult interface { // ContainerExport retrieves the raw contents of a container // and returns them as an [io.ReadCloser]. It's up to the caller // to close the stream. +// +// The underlying [io.ReadCloser] is automatically closed if the context is canceled, func (cli *Client) ContainerExport(ctx context.Context, containerID string, options ContainerExportOptions) (ContainerExportResult, error) { containerID, err := trimID("container", containerID) if err != nil { @@ -31,30 +33,15 @@ func (cli *Client) ContainerExport(ctx context.Context, containerID string, opti } return &containerExportResult{ - body: resp.Body, + ReadCloser: newCancelReadCloser(ctx, resp.Body), }, nil } type containerExportResult struct { - // body must be closed to avoid a resource leak - body io.ReadCloser + io.ReadCloser } var ( _ io.ReadCloser = (*containerExportResult)(nil) _ ContainerExportResult = (*containerExportResult)(nil) ) - -func (r *containerExportResult) Read(p []byte) (int, error) { - if r == nil || r.body == nil { - return 0, io.EOF - } - return r.body.Read(p) -} - -func (r *containerExportResult) Close() error { - if r == nil || r.body == nil { - return nil - } - return r.body.Close() -} diff --git a/client/container_export_test.go b/client/container_export_test.go index 2f3a86469e..6156987adb 100644 --- a/client/container_export_test.go +++ b/client/container_export_test.go @@ -1,7 +1,6 @@ package client import ( - "context" "io" "net/http" "testing" @@ -17,14 +16,14 @@ func TestContainerExportError(t *testing.T) { ) assert.NilError(t, err) - _, err = client.ContainerExport(context.Background(), "nothing", ContainerExportOptions{}) + _, err = client.ContainerExport(t.Context(), "nothing", ContainerExportOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal)) - _, err = client.ContainerExport(context.Background(), "", ContainerExportOptions{}) + _, err = client.ContainerExport(t.Context(), "", ContainerExportOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument)) assert.Check(t, is.ErrorContains(err, "value is empty")) - _, err = client.ContainerExport(context.Background(), " ", ContainerExportOptions{}) + _, err = client.ContainerExport(t.Context(), " ", ContainerExportOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument)) assert.Check(t, is.ErrorContains(err, "value is empty")) } @@ -40,10 +39,10 @@ func TestContainerExport(t *testing.T) { }), ) assert.NilError(t, err) - body, err := client.ContainerExport(context.Background(), "container_id", ContainerExportOptions{}) + res, err := client.ContainerExport(t.Context(), "container_id", ContainerExportOptions{}) assert.NilError(t, err) - defer body.Close() - content, err := io.ReadAll(body) + defer res.Close() + content, err := io.ReadAll(res) assert.NilError(t, err) assert.Check(t, is.Equal(string(content), "response")) } diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 7e1cc2a19e..14caa4cf44 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -103,11 +103,11 @@ func (s *DockerAPISuite) TestContainerAPIGetExport(c *testing.T) { assert.NilError(c, err) defer apiClient.Close() - body, err := apiClient.ContainerExport(testutil.GetContext(c), name, client.ContainerExportOptions{}) + res, err := apiClient.ContainerExport(testutil.GetContext(c), name, client.ContainerExportOptions{}) assert.NilError(c, err) - defer body.Close() + defer res.Close() found := false - for tarReader := tar.NewReader(body); ; { + for tarReader := tar.NewReader(res); ; { h, err := tarReader.Next() if errors.Is(err, io.EOF) { break diff --git a/integration/container/export_test.go b/integration/container/export_test.go index 24c65755f2..3c84f34fb7 100644 --- a/integration/container/export_test.go +++ b/integration/container/export_test.go @@ -71,6 +71,7 @@ func TestExportContainerAfterDaemonRestart(t *testing.T) { d.Restart(t) - _, err := c.ContainerExport(ctx, ctrID, client.ContainerExportOptions{}) + res, err := c.ContainerExport(ctx, ctrID, client.ContainerExportOptions{}) assert.NilError(t, err) + _ = res.Close() } diff --git a/integration/container/overlayfs_linux_test.go b/integration/container/overlayfs_linux_test.go index 802f04a5ca..447290dfb0 100644 --- a/integration/container/overlayfs_linux_test.go +++ b/integration/container/overlayfs_linux_test.go @@ -32,10 +32,10 @@ func TestNoOverlayfsWarningsAboutUndefinedBehaviors(t *testing.T) { return err }}, {name: "export", operation: func(*testing.T) error { - rc, err := apiClient.ContainerExport(ctx, cID, client.ContainerExportOptions{}) + res, err := apiClient.ContainerExport(ctx, cID, client.ContainerExportOptions{}) if err == nil { - defer rc.Close() - _, err = io.Copy(io.Discard, rc) + _, err = io.Copy(io.Discard, res) + _ = res.Close() } return err }}, @@ -48,8 +48,8 @@ func TestNoOverlayfsWarningsAboutUndefinedBehaviors(t *testing.T) { {name: "cp from container", operation: func(*testing.T) error { res, err := apiClient.CopyFromContainer(ctx, cID, client.CopyFromContainerOptions{SourcePath: "/file"}) if err == nil { - defer res.Content.Close() _, err = io.Copy(io.Discard, res.Content) + _ = res.Content.Close() } return err diff --git a/integration/plugin/authz/authz_plugin_test.go b/integration/plugin/authz/authz_plugin_test.go index 809975428b..0010336e12 100644 --- a/integration/plugin/authz/authz_plugin_test.go +++ b/integration/plugin/authz/authz_plugin_test.go @@ -363,13 +363,13 @@ func TestAuthZPluginEnsureLoadImportWorking(t *testing.T) { cID := container.Run(ctx, t, c) - responseReader, err := c.ContainerExport(ctx, cID, client.ContainerExportOptions{}) + res, err := c.ContainerExport(ctx, cID, client.ContainerExportOptions{}) assert.NilError(t, err) - defer responseReader.Close() + defer func() { _ = res.Close() }() file, err := os.Create(exportedImagePath) assert.NilError(t, err) - defer file.Close() - _, err = io.Copy(file, responseReader) + defer func() { _ = file.Close() }() + _, err = io.Copy(file, res) assert.NilError(t, err) err = imageImport(ctx, c, exportedImagePath) diff --git a/vendor/github.com/moby/moby/client/container_export.go b/vendor/github.com/moby/moby/client/container_export.go index d2929ee4b6..2d33efb7d8 100644 --- a/vendor/github.com/moby/moby/client/container_export.go +++ b/vendor/github.com/moby/moby/client/container_export.go @@ -19,6 +19,8 @@ type ContainerExportResult interface { // ContainerExport retrieves the raw contents of a container // and returns them as an [io.ReadCloser]. It's up to the caller // to close the stream. +// +// The underlying [io.ReadCloser] is automatically closed if the context is canceled, func (cli *Client) ContainerExport(ctx context.Context, containerID string, options ContainerExportOptions) (ContainerExportResult, error) { containerID, err := trimID("container", containerID) if err != nil { @@ -31,30 +33,15 @@ func (cli *Client) ContainerExport(ctx context.Context, containerID string, opti } return &containerExportResult{ - body: resp.Body, + ReadCloser: newCancelReadCloser(ctx, resp.Body), }, nil } type containerExportResult struct { - // body must be closed to avoid a resource leak - body io.ReadCloser + io.ReadCloser } var ( _ io.ReadCloser = (*containerExportResult)(nil) _ ContainerExportResult = (*containerExportResult)(nil) ) - -func (r *containerExportResult) Read(p []byte) (int, error) { - if r == nil || r.body == nil { - return 0, io.EOF - } - return r.body.Read(p) -} - -func (r *containerExportResult) Close() error { - if r == nil || r.body == nil { - return nil - } - return r.body.Close() -}