From bbe92af00542d87ab6feeef1be1be4c6dffcf121 Mon Sep 17 00:00:00 2001
From: "Lixia (Sylvia) Lei" <lixlei@microsoft.com>
Date: Mon, 7 Aug 2023 16:26:44 +0800
Subject: [PATCH] fix: pack should not set `artifactType` when config is
 specified (#565)

Related issue: #532
Related discussion:
https://github.com/oras-project/oras/issues/1011#issuecomment-1664614098

Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
---
 pack.go      | 12 +++++-------
 pack_test.go | 33 +++++++++++++++++++++++++++------
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/pack.go b/pack.go
index c5686242..a6c72e35 100644
--- a/pack.go
+++ b/pack.go
@@ -217,6 +217,11 @@ func packImageRC2(ctx context.Context, pusher content.Pusher, configMediaType st
 // If succeeded, returns a descriptor of the manifest.
 // Reference: https://github.com/opencontainers/image-spec/blob/v1.1.0-rc4/manifest.md#guidelines-for-artifact-usage
 func packImageRC4(ctx context.Context, pusher content.Pusher, artifactType string, layers []ocispec.Descriptor, opts PackOptions) (ocispec.Descriptor, error) {
+	if artifactType == "" && (opts.ConfigDescriptor == nil || opts.ConfigDescriptor.MediaType == ocispec.MediaTypeEmptyJSON) {
+		// artifactType MUST be set when config.mediaType is set to the empty value
+		return ocispec.Descriptor{}, ErrMissingArtifactType
+	}
+
 	var emptyBlobExists bool
 	var configDesc ocispec.Descriptor
 	if opts.ConfigDescriptor != nil {
@@ -232,13 +237,6 @@ func packImageRC4(ctx context.Context, pusher content.Pusher, artifactType strin
 		}
 		emptyBlobExists = true
 	}
-	if artifactType == "" {
-		if configDesc.MediaType == ocispec.MediaTypeEmptyJSON {
-			// artifactType MUST be set when config.mediaType is set to the empty value
-			return ocispec.Descriptor{}, ErrMissingArtifactType
-		}
-		artifactType = configDesc.MediaType
-	}
 
 	annotations, err := ensureAnnotationCreated(opts.ManifestAnnotations, ocispec.AnnotationCreated)
 	if err != nil {
diff --git a/pack_test.go b/pack_test.go
index 01c2b02b..b67580d6 100644
--- a/pack_test.go
+++ b/pack_test.go
@@ -678,6 +678,14 @@ func Test_Pack_ImageRC4_WithOptions(t *testing.T) {
 	}
 
 	// test Pack with ConfigDescriptor, but without artifactType
+	opts = PackOptions{
+		PackImageManifest:   true,
+		PackManifestType:    PackManifestTypeImageV1_1_0_RC4,
+		Subject:             &subjectDesc,
+		ConfigDescriptor:    &configDesc,
+		ConfigAnnotations:   configAnnotations,
+		ManifestAnnotations: annotations,
+	}
 	manifestDesc, err = Pack(ctx, s, "", layers, opts)
 	if err != nil {
 		t.Fatal("Oras.Pack() error =", err)
@@ -687,12 +695,11 @@ func Test_Pack_ImageRC4_WithOptions(t *testing.T) {
 		Versioned: specs.Versioned{
 			SchemaVersion: 2, // historical value. does not pertain to OCI or docker version
 		},
-		MediaType:    ocispec.MediaTypeImageManifest,
-		ArtifactType: configDesc.MediaType,
-		Subject:      &subjectDesc,
-		Config:       configDesc,
-		Layers:       layers,
-		Annotations:  annotations,
+		MediaType:   ocispec.MediaTypeImageManifest,
+		Subject:     &subjectDesc,
+		Config:      configDesc,
+		Layers:      layers,
+		Annotations: annotations,
 	}
 	expectedManifestBytes, err = json.Marshal(expectedManifest)
 	if err != nil {
@@ -783,6 +790,7 @@ func Test_Pack_ImageRC4_NoArtifactType(t *testing.T) {
 	s := memory.New()
 
 	ctx := context.Background()
+	// test no artifact type and no config
 	opts := PackOptions{
 		PackImageManifest: true,
 		PackManifestType:  PackManifestTypeImageV1_1_0_RC4,
@@ -791,6 +799,19 @@ func Test_Pack_ImageRC4_NoArtifactType(t *testing.T) {
 	if wantErr := ErrMissingArtifactType; !errors.Is(err, wantErr) {
 		t.Errorf("Oras.Pack() error = %v, wantErr = %v", err, wantErr)
 	}
+
+	// test no artifact type and config with media type empty
+	opts = PackOptions{
+		PackImageManifest: true,
+		PackManifestType:  PackManifestTypeImageV1_1_0_RC4,
+		ConfigDescriptor: &ocispec.Descriptor{
+			MediaType: ocispec.DescriptorEmptyJSON.MediaType,
+		},
+	}
+	_, err = Pack(ctx, s, "", nil, opts)
+	if wantErr := ErrMissingArtifactType; !errors.Is(err, wantErr) {
+		t.Errorf("Oras.Pack() error = %v, wantErr = %v", err, wantErr)
+	}
 }
 
 func Test_Pack_ImageRC4_NoLayer(t *testing.T) {