From 3cff7fafd140d8925f405340484fd050dec7318d Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Mon, 16 Mar 2026 10:47:57 -0700 Subject: [PATCH] Add encoding to os version and features Prevent os version or features containing characters which are used for formatting from creating invalid output. The character set to be encoded is small and likely rarely used, but include to prevent compatibility issues in the future or creating unintended limits on the values. Signed-off-by: Derek McGowan --- platforms.go | 46 ++++++++++++++++++--- platforms_test.go | 101 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 6 deletions(-) diff --git a/platforms.go b/platforms.go index 53e729f..54d5bc7 100644 --- a/platforms.go +++ b/platforms.go @@ -111,6 +111,7 @@ package platforms import ( "fmt" + "net/url" "path" "regexp" "runtime" @@ -123,7 +124,7 @@ import ( var ( specifierRe = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`) - osRe = regexp.MustCompile(`^([A-Za-z0-9_-]+)(?:\(([A-Za-z0-9_.-]*)((?:\+[A-Za-z0-9_.-]+)*)\))?$`) + osRe = regexp.MustCompile(`^([A-Za-z0-9_-]+)(?:\(([A-Za-z0-9_.%-]*)((?:\+[A-Za-z0-9_.%-]+)*)\))?$`) ) // Platform is a type alias for convenience, so there is no need to import image-spec package everywhere. @@ -240,9 +241,20 @@ func Parse(specifier string) (specs.Platform, error) { } p.OS = normalizeOS(osOptions[1]) - p.OSVersion = osOptions[2] + osVersion, err := decodeOSOption(osOptions[2]) + if err != nil { + return specs.Platform{}, fmt.Errorf("%q has an invalid OS version %q: %w", specifier, osOptions[2], err) + } + p.OSVersion = osVersion if osOptions[3] != "" { - p.OSFeatures = strings.Split(osOptions[3][1:], "+") + rawFeatures := strings.Split(osOptions[3][1:], "+") + p.OSFeatures = make([]string, len(rawFeatures)) + for i, f := range rawFeatures { + p.OSFeatures[i], err = decodeOSOption(f) + if err != nil { + return specs.Platform{}, fmt.Errorf("%q has an invalid OS feature %q: %w", specifier, f, err) + } + } } } else { if !specifierRe.MatchString(part) { @@ -327,14 +339,14 @@ func FormatAll(platform specs.Platform) string { return "unknown" } - osOptions := platform.OSVersion + osOptions := encodeOSOption(platform.OSVersion) features := platform.OSFeatures if !slices.IsSorted(features) { features = slices.Clone(features) slices.Sort(features) } - if len(features) > 0 { - osOptions += "+" + strings.Join(features, "+") + for _, f := range features { + osOptions += "+" + encodeOSOption(f) } if osOptions != "" { OSAndVersion := fmt.Sprintf("%s(%s)", platform.OS, osOptions) @@ -343,6 +355,28 @@ func FormatAll(platform specs.Platform) string { return path.Join(platform.OS, platform.Architecture, platform.Variant) } +// osOptionReplacer encodes characters in OS option values (version and +// features) that are ambiguous with the format syntax. The percent sign +// must be replaced first to avoid double-encoding. +var osOptionReplacer = strings.NewReplacer( + "%", "%25", + "+", "%2B", + "(", "%28", + ")", "%29", + "/", "%2F", +) + +func encodeOSOption(v string) string { + return osOptionReplacer.Replace(v) +} + +func decodeOSOption(v string) (string, error) { + if strings.Contains(v, "%") { + return url.PathUnescape(v) + } + return v, nil +} + // Normalize validates and translate the platform to the canonical value. // // For example, if "Aarch64" is encountered, we change it to "arm64" or if diff --git a/platforms_test.go b/platforms_test.go index c9e8326..dddecb8 100644 --- a/platforms_test.go +++ b/platforms_test.go @@ -391,6 +391,40 @@ func TestParseSelector(t *testing.T) { formatted: path.Join("linux(+erofs+unsorted)", defaultArch, defaultVariant), useV2Format: true, }, + { + input: "windows(10.0.17763%2Bbuild.42)", + expected: specs.Platform{ + OS: "windows", + OSVersion: "10.0.17763+build.42", + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("windows(10.0.17763%2Bbuild.42)", defaultArch, defaultVariant), + useV2Format: true, + }, + { + input: "windows(10.0.17763%2Bbuild.42+win32k)", + expected: specs.Platform{ + OS: "windows", + OSVersion: "10.0.17763+build.42", + OSFeatures: []string{"win32k"}, + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("windows(10.0.17763%2Bbuild.42+win32k)", defaultArch, defaultVariant), + useV2Format: true, + }, + { + input: "windows(50%25done)", + expected: specs.Platform{ + OS: "windows", + OSVersion: "50%done", + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("windows(50%25done)", defaultArch, defaultVariant), + useV2Format: true, + }, } { t.Run(testcase.input, func(t *testing.T) { if testcase.skip { @@ -446,6 +480,73 @@ func TestParseSelector(t *testing.T) { } } +func TestFormatAllEncoding(t *testing.T) { + for _, testcase := range []struct { + platform specs.Platform + expected string + }{ + { + platform: specs.Platform{OS: "windows", OSVersion: "10.0.17763+build.42", Architecture: "amd64"}, + expected: "windows(10.0.17763%2Bbuild.42)/amd64", + }, + { + platform: specs.Platform{OS: "windows", OSVersion: "10.0.17763+build.42", OSFeatures: []string{"win32k"}, Architecture: "amd64"}, + expected: "windows(10.0.17763%2Bbuild.42+win32k)/amd64", + }, + { + platform: specs.Platform{OS: "windows", OSVersion: "50%done", Architecture: "amd64"}, + expected: "windows(50%25done)/amd64", + }, + { + platform: specs.Platform{OS: "windows", OSVersion: "1.0(beta)", Architecture: "amd64"}, + expected: "windows(1.0%28beta%29)/amd64", + }, + { + platform: specs.Platform{OS: "windows", OSVersion: "a/b", Architecture: "amd64"}, + expected: "windows(a%2Fb)/amd64", + }, + { + // no special characters, no encoding needed + platform: specs.Platform{OS: "windows", OSVersion: "10.0.17763", Architecture: "amd64"}, + expected: "windows(10.0.17763)/amd64", + }, + { + // feature with + in the name + platform: specs.Platform{OS: "linux", OSFeatures: []string{"feat+v2"}, Architecture: "amd64"}, + expected: "linux(+feat%2Bv2)/amd64", + }, + { + // feature with % in the name + platform: specs.Platform{OS: "linux", OSFeatures: []string{"100%gpu"}, Architecture: "amd64"}, + expected: "linux(+100%25gpu)/amd64", + }, + { + // version and feature both with special characters + platform: specs.Platform{OS: "windows", OSVersion: "10.0+build", OSFeatures: []string{"feat+1"}, Architecture: "amd64"}, + expected: "windows(10.0%2Bbuild+feat%2B1)/amd64", + }, + } { + t.Run(testcase.expected, func(t *testing.T) { + formatted := FormatAll(testcase.platform) + if formatted != testcase.expected { + t.Fatalf("unexpected format: %q != %q", formatted, testcase.expected) + } + + // verify round-trip + reparsed, err := Parse(formatted) + if err != nil { + t.Fatalf("error parsing formatted output: %v", err) + } + if reparsed.OSVersion != testcase.platform.OSVersion { + t.Fatalf("OSVersion did not survive round trip: %q != %q", reparsed.OSVersion, testcase.platform.OSVersion) + } + if !reflect.DeepEqual(reparsed.OSFeatures, testcase.platform.OSFeatures) { + t.Fatalf("OSFeatures did not survive round trip: %v != %v", reparsed.OSFeatures, testcase.platform.OSFeatures) + } + }) + } +} + func TestParseSelectorInvalid(t *testing.T) { for _, testcase := range []struct { input string