Fix content copy to not ignore unexpected EOF

dmcgowan · dmcgowan · commit 2458afeb131b · 2021-09-09T10:01:51.000-07:00
Signed-off-by: Derek McGowan &lt;derek@mcg.dev&gt;
diff --git a/content/helpers.go b/content/helpers.go
@@ -144,9 +144,14 @@ func Copy(ctx context.Context, cw Writer, r io.Reader, size int64, expected dige
 		}
 	}
 
-	if _, err := copyWithBuffer(cw, r); err != nil {
+	copied, err := copyWithBuffer(cw, r)
+	if err != nil {
 		return errors.Wrap(err, "failed to copy")
 	}
+	if size != 0 && copied < size-ws.Offset {
+		// Short writes would return its own error, this indicates a read failure
+		return errors.Wrapf(io.ErrUnexpectedEOF, "failed to read expected number of bytes")
+	}
 
 	if err := cw.Commit(ctx, size, expected, opts...); err != nil {
 		if !errdefs.IsAlreadyExists(err) {
@@ -165,8 +170,15 @@ func CopyReaderAt(cw Writer, ra ReaderAt, n int64) error {
 		return err
 	}
 
-	_, err = copyWithBuffer(cw, io.NewSectionReader(ra, ws.Offset, n))
-	return err
+	copied, err := copyWithBuffer(cw, io.NewSectionReader(ra, ws.Offset, n))
+	if err != nil {
+		return errors.Wrap(err, "failed to copy")
+	}
+	if copied < n {
+		// Short writes would return its own error, this indicates a read failure
+		return errors.Wrap(io.ErrUnexpectedEOF, "failed to read expected number of bytes")
+	}
+	return nil
 }
 
 // CopyReader copies to a writer from a given reader, returning
diff --git a/content/helpers_test.go b/content/helpers_test.go
@@ -65,10 +65,11 @@ func TestCopy(t *testing.T) {
 		},
 		{
 			name:   "commit already exists",
-			source: defaultSource,
+			source: newCopySource("this already exists"),
 			writer: fakeWriter{commitFunc: func() error {
 				return errdefs.ErrAlreadyExists
 			}},
+			expected: "this already exists",
 		},
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -144,9 +144,14 @@ func Copy(ctx context.Context, cw Writer, r io.Reader, size int64, expected dige`
`144`	`144`	`}`
`145`	`145`	`}`
`146`	`146`
`147`		`- if _, err := copyWithBuffer(cw, r); err != nil {`
	`147`	`+ copied, err := copyWithBuffer(cw, r)`
	`148`	`+ if err != nil {`
`148`	`149`	`return errors.Wrap(err, "failed to copy")`
`149`	`150`	`}`
	`151`	`+ if size != 0 && copied < size-ws.Offset {`
	`152`	`+ // Short writes would return its own error, this indicates a read failure`
	`153`	`+ return errors.Wrapf(io.ErrUnexpectedEOF, "failed to read expected number of bytes")`
	`154`	`+ }`
`150`	`155`
`151`	`156`	`if err := cw.Commit(ctx, size, expected, opts...); err != nil {`
`152`	`157`	`if !errdefs.IsAlreadyExists(err) {`
`@@ -165,8 +170,15 @@ func CopyReaderAt(cw Writer, ra ReaderAt, n int64) error {`
`165`	`170`	`return err`
`166`	`171`	`}`
`167`	`172`
`168`		`- _, err = copyWithBuffer(cw, io.NewSectionReader(ra, ws.Offset, n))`
`169`		`- return err`
	`173`	`+ copied, err := copyWithBuffer(cw, io.NewSectionReader(ra, ws.Offset, n))`
	`174`	`+ if err != nil {`
	`175`	`+ return errors.Wrap(err, "failed to copy")`
	`176`	`+ }`
	`177`	`+ if copied < n {`
	`178`	`+ // Short writes would return its own error, this indicates a read failure`
	`179`	`+ return errors.Wrap(io.ErrUnexpectedEOF, "failed to read expected number of bytes")`
	`180`	`+ }`
	`181`	`+ return nil`
`170`	`182`	`}`
`171`	`183`
`172`	`184`	`// CopyReader copies to a writer from a given reader, returning`
Original file line number	Diff line number	Diff line change
`@@ -65,10 +65,11 @@ func TestCopy(t *testing.T) {`
`65`	`65`	`},`
`66`	`66`	`{`
`67`	`67`	`name: "commit already exists",`
`68`		`- source: defaultSource,`
	`68`	`+ source: newCopySource("this already exists"),`
`69`	`69`	`writer: fakeWriter{commitFunc: func() error {`
`70`	`70`	`return errdefs.ErrAlreadyExists`
`71`	`71`	`}},`
	`72`	`+ expected: "this already exists",`
`72`	`73`	`},`
`73`	`74`	`}`
`74`	`75`