From 1d47ef7b801369dc2ca257732ea91d39089ecebc Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Wed, 14 Mar 2018 16:55:45 -0400 Subject: [PATCH] Check media types when unmarshalling manifests When unmarshalling manifests from JSON, check that the MediaType field corresponds to the type that we are unmarshalling as. This makes sure that when we retrieve a manifest from the manifest store, it will have the same type as it was handled as before storing it in the manifest store. Signed-off-by: Owen W. Taylor --- manifest/manifestlist/manifestlist.go | 36 ++++++++--- manifest/manifestlist/manifestlist_test.go | 70 ++++++++++++++++++++-- manifest/ocischema/manifest.go | 5 ++ manifest/ocischema/manifest_test.go | 57 +++++++++++++++++- manifest/schema2/manifest.go | 6 ++ manifest/schema2/manifest_test.go | 57 +++++++++++++++++- 6 files changed, 214 insertions(+), 17 deletions(-) diff --git a/manifest/manifestlist/manifestlist.go b/manifest/manifestlist/manifestlist.go index 7be29714..11df4c25 100644 --- a/manifest/manifestlist/manifestlist.go +++ b/manifest/manifestlist/manifestlist.go @@ -38,6 +38,13 @@ func init() { return nil, distribution.Descriptor{}, err } + if m.MediaType != MediaTypeManifestList { + err = fmt.Errorf("mediaType in manifest list should be '%s' not '%s'", + MediaTypeManifestList, m.MediaType) + + return nil, distribution.Descriptor{}, err + } + dgst := digest.FromBytes(b) return m, distribution.Descriptor{Digest: dgst, Size: int64(len(b)), MediaType: MediaTypeManifestList}, err } @@ -53,6 +60,13 @@ func init() { return nil, distribution.Descriptor{}, err } + if m.MediaType != v1.MediaTypeImageIndex { + err = fmt.Errorf("mediaType in image index should be '%s' not '%s'", + v1.MediaTypeImageIndex, m.MediaType) + + return nil, distribution.Descriptor{}, err + } + dgst := digest.FromBytes(b) return m, distribution.Descriptor{Digest: dgst, Size: int64(len(b)), MediaType: v1.MediaTypeImageIndex}, err } @@ -130,15 +144,23 @@ type DeserializedManifestList struct { // DeserializedManifestList which contains the resulting manifest list // and its JSON representation. func FromDescriptors(descriptors []ManifestDescriptor) (*DeserializedManifestList, error) { - var m ManifestList + var mediaType string if len(descriptors) > 0 && descriptors[0].Descriptor.MediaType == v1.MediaTypeImageManifest { - m = ManifestList{ - Versioned: OCISchemaVersion, - } + mediaType = v1.MediaTypeImageIndex } else { - m = ManifestList{ - Versioned: SchemaVersion, - } + mediaType = MediaTypeManifestList + } + + return FromDescriptorsWithMediaType(descriptors, mediaType) +} + +// For testing purposes, it's useful to be able to specify the media type explicitly +func FromDescriptorsWithMediaType(descriptors []ManifestDescriptor, mediaType string) (*DeserializedManifestList, error) { + m := ManifestList{ + Versioned: manifest.Versioned{ + SchemaVersion: 2, + MediaType: mediaType, + }, } m.Manifests = make([]ManifestDescriptor, len(descriptors), len(descriptors)) diff --git a/manifest/manifestlist/manifestlist_test.go b/manifest/manifestlist/manifestlist_test.go index 720b911b..00b43900 100644 --- a/manifest/manifestlist/manifestlist_test.go +++ b/manifest/manifestlist/manifestlist_test.go @@ -38,7 +38,7 @@ var expectedManifestListSerialization = []byte(`{ ] }`) -func TestManifestList(t *testing.T) { +func makeTestManifestList(t *testing.T, mediaType string) ([]ManifestDescriptor, *DeserializedManifestList) { manifestDescriptors := []ManifestDescriptor{ { Descriptor: distribution.Descriptor{ @@ -65,11 +65,16 @@ func TestManifestList(t *testing.T) { }, } - deserialized, err := FromDescriptors(manifestDescriptors) + deserialized, err := FromDescriptorsWithMediaType(manifestDescriptors, mediaType) if err != nil { t.Fatalf("error creating DeserializedManifestList: %v", err) } + return manifestDescriptors, deserialized +} + +func TestManifestList(t *testing.T) { + manifestDescriptors, deserialized := makeTestManifestList(t, MediaTypeManifestList) mediaType, canonical, _ := deserialized.Payload() if mediaType != MediaTypeManifestList { @@ -160,7 +165,7 @@ var expectedOCIImageIndexSerialization = []byte(`{ ] }`) -func TestOCIImageIndex(t *testing.T) { +func makeTestOCIImageIndex(t *testing.T, mediaType string) ([]ManifestDescriptor, *DeserializedManifestList) { manifestDescriptors := []ManifestDescriptor{ { Descriptor: distribution.Descriptor{ @@ -196,11 +201,17 @@ func TestOCIImageIndex(t *testing.T) { }, } - deserialized, err := FromDescriptors(manifestDescriptors) + deserialized, err := FromDescriptorsWithMediaType(manifestDescriptors, mediaType) if err != nil { t.Fatalf("error creating DeserializedManifestList: %v", err) } + return manifestDescriptors, deserialized +} + +func TestOCIImageIndex(t *testing.T) { + manifestDescriptors, deserialized := makeTestOCIImageIndex(t, v1.MediaTypeImageIndex) + mediaType, canonical, _ := deserialized.Payload() if mediaType != v1.MediaTypeImageIndex { @@ -241,3 +252,54 @@ func TestOCIImageIndex(t *testing.T) { } } } + +func mediaTypeTest(t *testing.T, contentType string, mediaType string, shouldError bool) { + var m *DeserializedManifestList + if contentType == MediaTypeManifestList { + _, m = makeTestManifestList(t, mediaType) + } else { + _, m = makeTestOCIImageIndex(t, mediaType) + } + + _, canonical, err := m.Payload() + if err != nil { + t.Fatalf("error getting payload, %v", err) + } + + unmarshalled, descriptor, err := distribution.UnmarshalManifest( + contentType, + canonical) + + if shouldError { + if err == nil { + t.Fatalf("bad content type should have produced error") + } + } else { + if err != nil { + t.Fatalf("error unmarshaling manifest, %v", err) + } + + asManifest := unmarshalled.(*DeserializedManifestList) + if asManifest.MediaType != mediaType { + t.Fatalf("Bad media type '%v' as unmarshalled", asManifest.MediaType) + } + + if descriptor.MediaType != contentType { + t.Fatalf("Bad media type '%v' for descriptor", descriptor.MediaType) + } + + unmarshalledMediaType, _, _ := unmarshalled.Payload() + if unmarshalledMediaType != contentType { + t.Fatalf("Bad media type '%v' for payload", unmarshalledMediaType) + } + } +} + +func TestMediaTypes(t *testing.T) { + mediaTypeTest(t, MediaTypeManifestList, "", true) + mediaTypeTest(t, MediaTypeManifestList, MediaTypeManifestList, false) + mediaTypeTest(t, MediaTypeManifestList, MediaTypeManifestList+"XXX", true) + mediaTypeTest(t, v1.MediaTypeImageIndex, "", true) + mediaTypeTest(t, v1.MediaTypeImageIndex, v1.MediaTypeImageIndex, false) + mediaTypeTest(t, v1.MediaTypeImageIndex, v1.MediaTypeImageIndex+"XXX", true) +} diff --git a/manifest/ocischema/manifest.go b/manifest/ocischema/manifest.go index afab12bd..a6850fde 100644 --- a/manifest/ocischema/manifest.go +++ b/manifest/ocischema/manifest.go @@ -97,6 +97,11 @@ func (m *DeserializedManifest) UnmarshalJSON(b []byte) error { return err } + if manifest.MediaType != v1.MediaTypeImageManifest { + return fmt.Errorf("mediaType in manifest should be '%s' not '%s'", + v1.MediaTypeImageManifest, manifest.MediaType) + } + m.Manifest = manifest return nil diff --git a/manifest/ocischema/manifest_test.go b/manifest/ocischema/manifest_test.go index 749cb859..de53806d 100644 --- a/manifest/ocischema/manifest_test.go +++ b/manifest/ocischema/manifest_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/docker/distribution" + "github.com/docker/distribution/manifest" "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -36,9 +37,12 @@ var expectedManifestSerialization = []byte(`{ } }`) -func TestManifest(t *testing.T) { - manifest := Manifest{ - Versioned: SchemaVersion, +func makeTestManifest(mediaType string) Manifest { + return Manifest{ + Versioned: manifest.Versioned{ + SchemaVersion: 2, + MediaType: mediaType, + }, Config: distribution.Descriptor{ Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", Size: 985, @@ -55,6 +59,10 @@ func TestManifest(t *testing.T) { }, Annotations: map[string]string{"hot": "potato"}, } +} + +func TestManifest(t *testing.T) { + manifest := makeTestManifest(v1.MediaTypeImageManifest) deserialized, err := FromStruct(manifest) if err != nil { @@ -131,3 +139,46 @@ func TestManifest(t *testing.T) { t.Fatalf("unexpected annotation in reference: %s", references[1].Annotations["lettuce"]) } } + +func mediaTypeTest(t *testing.T, mediaType string, shouldError bool) { + manifest := makeTestManifest(mediaType) + + deserialized, err := FromStruct(manifest) + if err != nil { + t.Fatalf("error creating DeserializedManifest: %v", err) + } + + unmarshalled, descriptor, err := distribution.UnmarshalManifest( + v1.MediaTypeImageManifest, + deserialized.canonical) + + if shouldError { + if err == nil { + t.Fatalf("bad content type should have produced error") + } + } else { + if err != nil { + t.Fatalf("error unmarshaling manifest, %v", err) + } + + asManifest := unmarshalled.(*DeserializedManifest) + if asManifest.MediaType != mediaType { + t.Fatalf("Bad media type '%v' as unmarshalled", asManifest.MediaType) + } + + if descriptor.MediaType != v1.MediaTypeImageManifest { + t.Fatalf("Bad media type '%v' for descriptor", descriptor.MediaType) + } + + unmarshalledMediaType, _, _ := unmarshalled.Payload() + if unmarshalledMediaType != v1.MediaTypeImageManifest { + t.Fatalf("Bad media type '%v' for payload", unmarshalledMediaType) + } + } +} + +func TestMediaTypes(t *testing.T) { + mediaTypeTest(t, "", true) + mediaTypeTest(t, v1.MediaTypeImageManifest, false) + mediaTypeTest(t, v1.MediaTypeImageManifest+"XXX", true) +} diff --git a/manifest/schema2/manifest.go b/manifest/schema2/manifest.go index 26a6a334..8adbbd0f 100644 --- a/manifest/schema2/manifest.go +++ b/manifest/schema2/manifest.go @@ -116,6 +116,12 @@ func (m *DeserializedManifest) UnmarshalJSON(b []byte) error { return err } + if manifest.MediaType != MediaTypeManifest { + return fmt.Errorf("mediaType in manifest should be '%s' not '%s'", + MediaTypeManifest, manifest.MediaType) + + } + m.Manifest = manifest return nil diff --git a/manifest/schema2/manifest_test.go b/manifest/schema2/manifest_test.go index 86226606..912dda9a 100644 --- a/manifest/schema2/manifest_test.go +++ b/manifest/schema2/manifest_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/docker/distribution" + "github.com/docker/distribution/manifest" ) var expectedManifestSerialization = []byte(`{ @@ -26,9 +27,12 @@ var expectedManifestSerialization = []byte(`{ ] }`) -func TestManifest(t *testing.T) { - manifest := Manifest{ - Versioned: SchemaVersion, +func makeTestManifest(mediaType string) Manifest { + return Manifest{ + Versioned: manifest.Versioned{ + SchemaVersion: 2, + MediaType: mediaType, + }, Config: distribution.Descriptor{ Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", Size: 985, @@ -42,6 +46,10 @@ func TestManifest(t *testing.T) { }, }, } +} + +func TestManifest(t *testing.T) { + manifest := makeTestManifest(MediaTypeManifest) deserialized, err := FromStruct(manifest) if err != nil { @@ -109,3 +117,46 @@ func TestManifest(t *testing.T) { t.Fatalf("unexpected size in reference: %d", references[0].Size) } } + +func mediaTypeTest(t *testing.T, mediaType string, shouldError bool) { + manifest := makeTestManifest(mediaType) + + deserialized, err := FromStruct(manifest) + if err != nil { + t.Fatalf("error creating DeserializedManifest: %v", err) + } + + unmarshalled, descriptor, err := distribution.UnmarshalManifest( + MediaTypeManifest, + deserialized.canonical) + + if shouldError { + if err == nil { + t.Fatalf("bad content type should have produced error") + } + } else { + if err != nil { + t.Fatalf("error unmarshaling manifest, %v", err) + } + + asManifest := unmarshalled.(*DeserializedManifest) + if asManifest.MediaType != mediaType { + t.Fatalf("Bad media type '%v' as unmarshalled", asManifest.MediaType) + } + + if descriptor.MediaType != MediaTypeManifest { + t.Fatalf("Bad media type '%v' for descriptor", descriptor.MediaType) + } + + unmarshalledMediaType, _, _ := unmarshalled.Payload() + if unmarshalledMediaType != MediaTypeManifest { + t.Fatalf("Bad media type '%v' for payload", unmarshalledMediaType) + } + } +} + +func TestMediaTypes(t *testing.T) { + mediaTypeTest(t, "", true) + mediaTypeTest(t, MediaTypeManifest, false) + mediaTypeTest(t, MediaTypeManifest+"XXX", true) +}