fix: improve errors for ReadAll and loadIndexfile (#526)

sparr · shizhMSFT · commit f3d7906854cb · 2023-07-04T15:42:21.000+08:00
NewWithContext refers to both the `oci-layout` and `index.json` as "OCI Image Layout", this PR fixes the latter. ReadAll is a deeper change that will improve error reporting for a lot of cases, at the cost of error volume. Some or all of that change could happen higher up to satisfy this particular issue. I think the expected/actual size output belongs here. There seem to be more than a few code paths in this and other repos that end up calling ReadAll multiple times without reporting which call produced an error so I think it's appropriate to also output the digest here, but that's less compelling. The Verify comment clears up confusion I encountered while working on this issue. "verifies size" implies checking for both too big AND too small, but it only checks if the file is bigger (or reader is longer) than expected. Fixes #432 Signed-off-by: Clarence "Sparr" Risher <clrnc@amazon.com>
diff --git a/content/oci/oci.go b/content/oci/oci.go
@@ -102,7 +102,7 @@ func NewWithContext(ctx context.Context, root string) (*Store, error) {
 		return nil, fmt.Errorf("invalid OCI Image Layout: %w", err)
 	}
 	if err := store.loadIndexFile(ctx); err != nil {
-		return nil, fmt.Errorf("invalid OCI Image Layout: %w", err)
+		return nil, fmt.Errorf("invalid OCI Image Index: %w", err)
 	}
 
 	return store, nil
diff --git a/content/oci/readonlyoci.go b/content/oci/readonlyoci.go
@@ -57,7 +57,7 @@ func NewFromFS(ctx context.Context, fsys fs.FS) (*ReadOnlyStore, error) {
 		return nil, fmt.Errorf("invalid OCI Image Layout: %w", err)
 	}
 	if err := store.loadIndexFile(ctx); err != nil {
-		return nil, fmt.Errorf("invalid OCI Image Layout: %w", err)
+		return nil, fmt.Errorf("invalid OCI Image Index: %w", err)
 	}
 
 	return store, nil
diff --git a/content/reader.go b/content/reader.go
@@ -70,7 +70,7 @@ func (vr *VerifyReader) Read(p []byte) (n int, err error) {
 	return
 }
 
-// Verify verifies the read content against the size and the digest.
+// Verify checks for remaining unread content and verifies the read content against the digest
 func (vr *VerifyReader) Verify() error {
 	if vr.verified {
 		return nil
@@ -120,7 +120,10 @@ func ReadAll(r io.Reader, desc ocispec.Descriptor) ([]byte, error) {
 	buf := make([]byte, desc.Size)
 
 	vr := NewVerifyReader(r, desc)
-	if _, err := io.ReadFull(vr, buf); err != nil {
+	if n, err := io.ReadFull(vr, buf); err != nil {
+		if errors.Is(err, io.ErrUnexpectedEOF) {
+			return nil, fmt.Errorf("read failed: expected content size of %d, got %d, for digest %s: %w", desc.Size, n, desc.Digest.String(), err)
+		}
 		return nil, fmt.Errorf("read failed: %w", err)
 	}
 	if err := vr.Verify(); err != nil {

Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,7 @@ func NewWithContext(ctx context.Context, root string) (*Store, error) {`
`102`	`102`	`return nil, fmt.Errorf("invalid OCI Image Layout: %w", err)`
`103`	`103`	`}`
`104`	`104`	`if err := store.loadIndexFile(ctx); err != nil {`
`105`		`- return nil, fmt.Errorf("invalid OCI Image Layout: %w", err)`
	`105`	`+ return nil, fmt.Errorf("invalid OCI Image Index: %w", err)`
`106`	`106`	`}`
`107`	`107`
`108`	`108`	`return store, nil`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ func NewFromFS(ctx context.Context, fsys fs.FS) (*ReadOnlyStore, error) {`
`57`	`57`	`return nil, fmt.Errorf("invalid OCI Image Layout: %w", err)`
`58`	`58`	`}`
`59`	`59`	`if err := store.loadIndexFile(ctx); err != nil {`
`60`		`- return nil, fmt.Errorf("invalid OCI Image Layout: %w", err)`
	`60`	`+ return nil, fmt.Errorf("invalid OCI Image Index: %w", err)`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`return store, nil`