fix!: fix colon in media type for windows (#698)

Fixes #697 

Signed-off-by: Billy Zha <jinzha1@microsoft.com>

cmd/oras/file.go

@@ -22,12 +22,16 @@ import (
 
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2/content/file"
+	"oras.land/oras/cmd/oras/internal/fileref"
 )
 
 func loadFiles(ctx context.Context, store *file.Store, annotations map[string]map[string]string, fileRefs []string, verbose bool) ([]ocispec.Descriptor, error) {
 	var files []ocispec.Descriptor
 	for _, fileRef := range fileRefs {
-		filename, mediaType := parseFileReference(fileRef, "")
+		filename, mediaType, err := fileref.Parse(fileRef, "")
+		if err != nil {
+			return nil, err
+		}
 
 		// get shortest absolute path as unique name
 		name := filepath.Clean(filename)

cmd/oras/file_unix.go → cmd/oras/internal/fileref/unix.go

@@ -15,16 +15,23 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package main
+package fileref
 
-import "strings"
+import (
+	"fmt"
+	"strings"
+)
 
-// parseFileReference parses file reference on unix.
-func parseFileReference(reference string, mediaType string) (filePath, mediatype string) {
+// Parse parses file reference on unix.
+func Parse(reference string, defaultMediaType string) (filePath, mediaType string, err error) {
 	i := strings.LastIndex(reference, ":")
 	if i < 0 {
-		return reference, mediaType
+		filePath, mediaType = reference, defaultMediaType
+	} else {
+		filePath, mediaType = reference[:i], reference[i+1:]
 	}
-	return reference[:i], reference[i+1:]
-
+	if filePath == "" {
+		return "", "", fmt.Errorf("found empty file path in %q", reference)
+	}
+	return filePath, mediaType, nil
 }

cmd/oras/internal/fileref/unix_test.go

@@ -0,0 +1,90 @@
+//go:build !windows
+
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fileref
+
+import "testing"
+
+func Test_ParseFileReference(t *testing.T) {
+	type args struct {
+		reference string
+		mediaType string
+	}
+	tests := []struct {
+		name          string
+		args          args
+		wantFilePath  string
+		wantMediatype string
+	}{
+		{"file name and media type", args{"az:b", ""}, "az", "b"},
+		{"file name and empty media type", args{"az:", ""}, "az", ""},
+		{"file name and default media type", args{"az", "c"}, "az", "c"},
+		{"file name and media type, default type ignored", args{"az:b", "c"}, "az", "b"},
+		{"file name and empty media type, default type ignored", args{"az:", "c"}, "az", ""},
+		{"colon file name and media type", args{"az:b:c", "d"}, "az:b", "c"},
+		{"colon file name and empty media type", args{"az:b:", "c"}, "az:b", ""},
+		{"colon-prefix file name and media type", args{":az:b:c", "d"}, ":az:b", "c"},
+
+		{"pure colon file name and media type", args{"::a", "b"}, ":", "a"},
+		{"pure colon file name and empty media type", args{"::", "a"}, ":", ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotFilePath, gotMediatype, _ := Parse(tt.args.reference, tt.args.mediaType)
+			if gotFilePath != tt.wantFilePath {
+				t.Errorf("Parse() gotFilePath = %v, want %v", gotFilePath, tt.wantFilePath)
+			}
+			if gotMediatype != tt.wantMediatype {
+				t.Errorf("Parse() gotMediatype = %v, want %v", gotMediatype, tt.wantMediatype)
+			}
+		})
+	}
+}
+
+func TestParse(t *testing.T) {
+	type args struct {
+		reference string
+		mediaType string
+	}
+	tests := []struct {
+		name          string
+		args          args
+		wantFilePath  string
+		wantMediatype string
+		wantErr       bool
+	}{
+
+		{"no input", args{"", ""}, "", "", true},
+		{"empty file name and media type", args{":", ""}, "", "", true},
+		{"empty file name with media type", args{":a", "b"}, "", "", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotFilePath, gotMediatype, err := Parse(tt.args.reference, tt.args.mediaType)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if gotFilePath != tt.wantFilePath {
+				t.Errorf("Parse() gotFilePath = %v, want %v", gotFilePath, tt.wantFilePath)
+			}
+			if gotMediatype != tt.wantMediatype {
+				t.Errorf("Parse() gotMediatype = %v, want %v", gotMediatype, tt.wantMediatype)
+			}
+		})
+	}
+}

cmd/oras/internal/fileref/windows.go

@@ -0,0 +1,46 @@
+//go:build windows
+
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fileref
+
+import (
+	"fmt"
+	"strings"
+	"unicode"
+)
+
+// Parse parses file reference on windows.
+func Parse(reference string, defaultMediaType string) (filePath, mediaType string, err error) {
+	filePath, mediaType = doParse(reference, defaultMediaType)
+	if filePath == "" {
+		return "", "", fmt.Errorf("found empty file path in %q", reference)
+	}
+	if strings.ContainsAny(filePath, `<>:"|?*`) {
+		// Reference: https://learn.microsoft.com/windows/win32/fileio/naming-a-file#naming-conventions
+		return "", "", fmt.Errorf("reserved characters found in the file path: %s", filePath)
+	}
+	return filePath, mediaType, nil
+}
+
+func doParse(reference string, mediaType string) (filePath, mediatype string) {
+	i := strings.LastIndex(reference, ":")
+	if i < 0 || (i == 1 && len(reference) > 2 && unicode.IsLetter(rune(reference[0])) && reference[2] == '\\') {
+		// Relative file path with disk prefix is NOT supported, e.g. `c:file1`
+		return reference, mediaType
+	}
+	return reference[:i], reference[i+1:]
+}

cmd/oras/internal/fileref/windows_test.go

@@ -0,0 +1,117 @@
+//go:build windows
+
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fileref
+
+import (
+	"testing"
+)
+
+func Test_doParse(t *testing.T) {
+	type args struct {
+		reference string
+		mediaType string
+	}
+	tests := []struct {
+		name          string
+		args          args
+		wantFilePath  string
+		wantMediatype string
+	}{
+		{"file name and media type", args{"az:b", ""}, "az", "b"},
+		{"file name and empty media type", args{"az:", ""}, "az", ""},
+		{"file name and default media type", args{"az", "c"}, "az", "c"},
+		{"file name and media type, default type ignored", args{"az:b", "c"}, "az", "b"},
+		{"file name and empty media type, default type ignored", args{"az:", "c"}, "az", ""},
+
+		{"empty file name and media type", args{":a", "b"}, "", "a"},
+		{"empty file name and empty media type", args{":", "a"}, "", ""},
+		{"empty name and default media type", args{"", "a"}, "", "a"},
+
+		{"colon file name and media type", args{"az:b:c", "d"}, "az:b", "c"},
+		{"colon file name and empty media type", args{"az:b:", "c"}, "az:b", ""},
+		{"colon-prefix file name and media type", args{":az:b:c", "d"}, ":az:b", "c"},
+
+		{"pure colon file name and media type", args{"::a", "b"}, ":", "a"},
+		{"pure colon file name and empty media type", args{"::", "a"}, ":", ""},
+
+		{"windows file name1 and default type", args{`a:\b`, "c"}, `a:\b`, "c"},
+		{"windows file name2 and default type", args{`z:b`, "c"}, `z`, "b"},
+		{"windows file name and media type", args{`a:\b:c`, "d"}, `a:\b`, "c"},
+		{"windows file name and empty media type", args{`a:\b:`, "c"}, `a:\b`, ""},
+		{"numeric file name and media type", args{`1:\a`, "b"}, `1`, `\a`},
+		{"non-windows file name and media type", args{`ab:\c`, ""}, `ab`, `\c`},
+		{"non-windows file name and media type, default type ignored", args{`1:\a`, "b"}, `1`, `\a`},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotFilePath, gotMediatype := doParse(tt.args.reference, tt.args.mediaType)
+			if gotFilePath != tt.wantFilePath {
+				t.Errorf("doParse() gotFilePath = %v, want %v", gotFilePath, tt.wantFilePath)
+			}
+			if gotMediatype != tt.wantMediatype {
+				t.Errorf("doParse() gotMediatype = %v, want %v", gotMediatype, tt.wantMediatype)
+			}
+		})
+	}
+}
+
+func TestParse(t *testing.T) {
+	type args struct {
+		reference string
+		mediaType string
+	}
+	tests := []struct {
+		name          string
+		args          args
+		wantFilePath  string
+		wantMediatype string
+		wantErr       bool
+	}{
+		{"no input", args{"", ""}, "", "", true},
+		{"empty file name", args{":", ""}, "", "", true},
+		{"reserved character1 in file name", args{"<", "a"}, "", "", true},
+		{"reserved character2 in file name", args{">", "a"}, "", "", true},
+		{"reserved character3 in file name", args{"*", "a"}, "", "", true},
+		{"reserved character4 in file name", args{`"`, "a"}, "", "", true},
+		{"reserved character5 in file name", args{"|", "a"}, "", "", true},
+		{"reserved character6 in file name", args{"?", "a"}, "", "", true},
+		{"empty file name, with media type", args{":", "a"}, "", "", true},
+		{"reserved character1 in file name, with media type", args{"<:", "a"}, "", "", true},
+		{"reserved character2 in file name, with media type", args{">:", "a"}, "", "", true},
+		{"reserved character3 in file name, with media type", args{"*:", "a"}, "", "", true},
+		{"reserved character4 in file name, with media type", args{`":`, "a"}, "", "", true},
+		{"reserved character5 in file name, with media type", args{"|:", "a"}, "", "", true},
+		{"reserved character6 in file name, with media type", args{"?:", "a"}, "", "", true},
+		{"reserved character7 in file name, with media type", args{"::", "a"}, "", "", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotFilePath, gotMediatype, err := Parse(tt.args.reference, tt.args.mediaType)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if gotFilePath != tt.wantFilePath {
+				t.Errorf("Parse() gotFilePath = %v, want %v", gotFilePath, tt.wantFilePath)
+			}
+			if gotMediatype != tt.wantMediatype {
+				t.Errorf("Parse() gotMediatype = %v, want %v", gotMediatype, tt.wantMediatype)
+			}
+		})
+	}
+}

cmd/oras/pull.go

@@ -28,6 +28,7 @@ import (
 	"oras.land/oras-go/v2/content/file"
 	"oras.land/oras/cmd/oras/internal/display"
 	"oras.land/oras/cmd/oras/internal/errors"
+	"oras.land/oras/cmd/oras/internal/fileref"
 	"oras.land/oras/cmd/oras/internal/option"
 	"oras.land/oras/internal/graph"
 )
@@ -117,7 +118,13 @@ func runPull(opts pullOptions) error {
 	// Copy Options
 	copyOptions := oras.DefaultCopyOptions
 	copyOptions.Concurrency = opts.concurrency
-	configPath, configMediaType := parseFileReference(opts.ManifestConfigRef, "")
+	var configPath, configMediaType string
+	if opts.ManifestConfigRef != "" {
+		configPath, configMediaType, err = fileref.Parse(opts.ManifestConfigRef, "")
+		if err != nil {
+			return err
+		}
+	}
 	if targetPlatform != nil {
 		copyOptions.WithTargetPlatform(targetPlatform)
 	}

cmd/oras/push.go

@@ -32,6 +32,7 @@ import (
 	"oras.land/oras-go/v2/registry/remote"
 	"oras.land/oras-go/v2/registry/remote/errcode"
 	"oras.land/oras/cmd/oras/internal/display"
+	"oras.land/oras/cmd/oras/internal/fileref"
 	"oras.land/oras/cmd/oras/internal/option"
 )
 
@@ -129,7 +130,10 @@ func runPush(opts pushOptions) error {
 	defer store.Close()
 	store.AllowPathTraversalOnWrite = opts.PathValidationDisabled
 	if opts.manifestConfigRef != "" {
-		path, cfgMediaType := parseFileReference(opts.manifestConfigRef, oras.MediaTypeUnknownConfig)
+		path, cfgMediaType, err := fileref.Parse(opts.manifestConfigRef, oras.MediaTypeUnknownConfig)
+		if err != nil {
+			return err
+		}
 		desc, err := store.Add(ctx, option.AnnotationConfig, cfgMediaType, path)
 		if err != nil {
 			return err