reject null manifests

jchorl · jchorl · commit 654f854faecb · 2021-02-02T09:24:53.000-08:00
Signed-off-by: Josh Chorlton &lt;jchorlton@gmail.com&gt;
diff --git a/daemon/images/image_exporter.go b/daemon/images/image_exporter.go
@@ -17,7 +17,7 @@ func (i *ImageService) ExportImage(names []string, outStream io.Writer) error {
 }
 
 // LoadImage uploads a set of images into the repository. This is the
-// complement of ImageExport.  The input stream is an uncompressed tar
+// complement of ExportImage.  The input stream is an uncompressed tar
 // ball containing images and metadata.
 func (i *ImageService) LoadImage(inTar io.ReadCloser, outStream io.Writer, quiet bool) error {
 	imageExporter := tarexport.NewTarExporter(i.imageStore, i.layerStores, i.referenceStore, i)
diff --git a/image/tarexport/load.go b/image/tarexport/load.go
@@ -63,6 +63,10 @@ func (l *tarexporter) Load(inTar io.ReadCloser, outStream io.Writer, quiet bool)
 		return err
 	}
 
+	if err := validateManifest(manifest); err != nil {
+		return err
+	}
+
 	var parentLinks []parentLink
 	var imageIDsStr string
 	var imageRefCount int
@@ -430,3 +434,13 @@ func checkCompatibleOS(imageOS string) error {
 
 	return system.ValidatePlatform(p)
 }
+
+func validateManifest(manifest []manifestItem) error {
+	// a nil manifest usually indicates a bug, so don't just silently fail.
+	// if someone really needs to pass an empty manifest, they can pass [].
+	if manifest == nil {
+		return errors.New("invalid manifest, manifest cannot be null (but can be [])")
+	}
+
+	return nil
+}
diff --git a/image/tarexport/load_test.go b/image/tarexport/load_test.go
@@ -0,0 +1,37 @@
+package tarexport
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+)
+
+func TestValidateManifest(t *testing.T) {
+	cases := map[string]struct {
+		manifest    []manifestItem
+		valid       bool
+		errContains string
+	}{
+		"nil": {
+			manifest:    nil,
+			valid:       false,
+			errContains: "manifest cannot be null",
+		},
+		"non-nil": {
+			manifest: []manifestItem{},
+			valid:    true,
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			err := validateManifest(tc.manifest)
+			if tc.valid {
+				assert.Check(t, is.Nil(err))
+			} else {
+				assert.Check(t, is.ErrorContains(err, tc.errContains))
+			}
+		})
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ func (i *ImageService) ExportImage(names []string, outStream io.Writer) error {`
`17`	`17`	`}`
`18`	`18`
`19`	`19`	`// LoadImage uploads a set of images into the repository. This is the`
`20`		`-// complement of ImageExport. The input stream is an uncompressed tar`
	`20`	`+// complement of ExportImage. The input stream is an uncompressed tar`
`21`	`21`	`// ball containing images and metadata.`
`22`	`22`	`func (i *ImageService) LoadImage(inTar io.ReadCloser, outStream io.Writer, quiet bool) error {`
`23`	`23`	`imageExporter := tarexport.NewTarExporter(i.imageStore, i.layerStores, i.referenceStore, i)`