copy: remove wrapping io.NopCloser from push writer pipe

jedevc · jedevc · commit 2a25c085b21e · 2024-03-04T12:58:51.000Z
io.Pipe produces a PipeReader and a PipeWriter - a close on the write
side, causes an error on both the read and write sides, while a close on
the read side causes an error on only the read side. Previously, we
explicitly prohibited closing from the read side.

However, http.Request.Body requires that "calling Close should unblock a
Read waiting for input". Our reader will not do this - calling close
becomes a no-op. This can cause a deadlock because client.Do may never
terminate in some circumstances.

We need the Reader side to close its side of the pipe as well, which it
already does using the go standard library - otherwise, we can hang
forever, writing to a pipe that will never be closed.

Allowing the requester to close the body should be safe - we never reuse
the same reader between requests, as the result of body() will never be
reused by the guarantees of the standard library.

Signed-off-by: Justin Chadwell &lt;me@jedevc.com&gt;
diff --git a/remotes/docker/pusher.go b/remotes/docker/pusher.go
@@ -284,7 +284,7 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str
 	req.body = func() (io.ReadCloser, error) {
 		pr, pw := io.Pipe()
 		pushw.setPipe(pw)
-		return io.NopCloser(pr), nil
+		return pr, nil
 	}
 	req.size = desc.Size
 

Original file line number	Diff line number	Diff line change
`@@ -284,7 +284,7 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str`
`284`	`284`	`req.body = func() (io.ReadCloser, error) {`
`285`	`285`	`pr, pw := io.Pipe()`
`286`	`286`	`pushw.setPipe(pw)`
`287`		`- return io.NopCloser(pr), nil`
	`287`	`+ return pr, nil`
`288`	`288`	`}`
`289`	`289`	`req.size = desc.Size`
`290`	`290`