From 75663cddc20b727338f95a89ebac40fea7a1de3e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 26 Mar 2026 15:56:07 +0100 Subject: [PATCH 1/7] gofumpt code Signed-off-by: Sebastiaan van Stijn --- cpuinfo_linux.go | 2 -- cpuinfo_linux_test.go | 3 --- cpuinfo_other.go | 1 - platform_windows_compat_test.go | 1 - 4 files changed, 7 deletions(-) diff --git a/cpuinfo_linux.go b/cpuinfo_linux.go index 98c7001..06da8b9 100644 --- a/cpuinfo_linux.go +++ b/cpuinfo_linux.go @@ -45,7 +45,6 @@ func getMachineArch() (string, error) { // So we don't need to access the ARM registers to detect platform information // by ourselves. We can just parse these information from /proc/cpuinfo func getCPUInfo(pattern string) (info string, err error) { - cpuinfo, err := os.Open("/proc/cpuinfo") if err != nil { return "", err @@ -75,7 +74,6 @@ func getCPUInfo(pattern string) (info string, err error) { // getCPUVariantFromArch get CPU variant from arch through a system call func getCPUVariantFromArch(arch string) (string, error) { - var variant string arch = strings.ToLower(arch) diff --git a/cpuinfo_linux_test.go b/cpuinfo_linux_test.go index ecb1150..59a8198 100644 --- a/cpuinfo_linux_test.go +++ b/cpuinfo_linux_test.go @@ -46,7 +46,6 @@ func TestCPUVariant(t *testing.T) { } func TestGetCPUVariantFromArch(t *testing.T) { - for _, testcase := range []struct { name string input string @@ -127,13 +126,11 @@ func TestGetCPUVariantFromArch(t *testing.T) { t.Fatalf("Expect to get variant: %v, however %v returned", testcase.output, variant) } } - } else { if !errors.Is(err, testcase.expectedErr) { t.Fatalf("Expect to get error: %v, however error %v returned", testcase.expectedErr, err) } } }) - } } diff --git a/cpuinfo_other.go b/cpuinfo_other.go index 5bbfef7..b8c7a4b 100644 --- a/cpuinfo_other.go +++ b/cpuinfo_other.go @@ -24,7 +24,6 @@ import ( ) func getCPUVariant() (string, error) { - var variant string switch runtime.GOOS { diff --git a/platform_windows_compat_test.go b/platform_windows_compat_test.go index 7a506a4..1a05075 100644 --- a/platform_windows_compat_test.go +++ b/platform_windows_compat_test.go @@ -183,5 +183,4 @@ func Test_PlatformOrder(t *testing.T) { } }) } - } From 042728d8c89bb01c02e934a842122e121f097041 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 26 Mar 2026 17:49:07 +0100 Subject: [PATCH 2/7] add benchmark for Parse, FormatAll Signed-off-by: Sebastiaan van Stijn --- platforms_test.go | 144 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/platforms_test.go b/platforms_test.go index dddecb8..d41ec8e 100644 --- a/platforms_test.go +++ b/platforms_test.go @@ -20,6 +20,8 @@ import ( "path" "reflect" "runtime" + "strconv" + "strings" "testing" specs "github.com/opencontainers/image-spec/specs-go/v1" @@ -578,6 +580,20 @@ func TestParseSelectorInvalid(t *testing.T) { } } +func TestFormatAllSkipsEmptyOSFeatures(t *testing.T) { + p := specs.Platform{ + OS: "linux", + Architecture: "amd64", + OSFeatures: []string{"", "gpu", "", "simd"}, + } + + formatted := FormatAll(p) + expected := "linux(+gpu+simd)/amd64" + if formatted != expected { + t.Fatalf("unexpected format: %q != %q", formatted, expected) + } +} + func FuzzPlatformsParse(f *testing.F) { f.Add("linux/amd64") f.Fuzz(func(t *testing.T, s string) { @@ -587,3 +603,131 @@ func FuzzPlatformsParse(f *testing.F) { } }) } + +func BenchmarkParseOSOptions(b *testing.B) { + maxFeatures := 16 + + benchmarks := []struct { + doc string + input string + }{ + { + doc: "valid windows version and feature", + input: "windows(10.0.17763+win32k)/amd64", + }, + { + doc: "valid but lengthy features", + input: "linux(+" + strings.Repeat("+feature", maxFeatures) + ")/amd64", + }, + { + doc: "exploding plus chain", + input: "linux(" + strings.Repeat("+", 64*1024) + ")/amd64", + }, + { + doc: "kernel config feature blob", + input: "linux(+CONFIG_" + strings.Repeat("FOO=y_", 16*1024) + "BAR)/amd64", + }, + } + + b.ReportAllocs() + b.ResetTimer() + for _, bm := range benchmarks { + b.Run(bm.doc, func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = Parse(bm.input) + } + }) + } +} + +func BenchmarkFormatAllOSFeatures(b *testing.B) { + maxFeatures := 16 + + benchmarks := []struct { + doc string + platform specs.Platform + }{ + { + doc: "plain linux amd64", + platform: specs.Platform{ + OS: "linux", + Architecture: "amd64", + }, + }, + { + doc: "windows version and feature", + platform: specs.Platform{ + OS: "windows", + OSVersion: "10.0.17763", + OSFeatures: []string{"win32k"}, + Architecture: "amd64", + }, + }, + { + doc: "valid but lengthy features", + platform: specs.Platform{ + OS: "linux", + OSFeatures: func() (out []string) { + for i := 0; i <= maxFeatures; i++ { + out = append(out, "feature") + } + return out + }(), + Architecture: "amd64", + }, + }, + { + doc: "skips empty features", + platform: specs.Platform{ + OS: "linux", + OSFeatures: []string{"", "gpu", "", "simd"}, + Architecture: "amd64", + }, + }, + { + doc: "kernel config feature blob", + platform: specs.Platform{ + OS: "linux", + OSFeatures: []string{"CONFIG_" + strings.Repeat("FOO_", 16*1024) + "BAR"}, + Architecture: "amd64", + }, + }, + { + doc: "many kernel config features with empties", + platform: specs.Platform{ + OS: "linux", + OSFeatures: func() []string { + n := 1024 + out := make([]string, n) + for i := range out { + if i%10 == 0 { + out[i] = "" // simulate bad data + } else { + out[i] = "CONFIG_FOO_" + strconv.Itoa(i) + } + } + return out + }(), + Architecture: "amd64", + }, + }, + { + doc: "too many features", + platform: specs.Platform{ + OS: "linux", + OSFeatures: make([]string, maxFeatures+1), + Architecture: "amd64", + }, + }, + } + + b.ReportAllocs() + b.ResetTimer() + for _, bm := range benchmarks { + b.Run(bm.doc, func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = FormatAll(bm.platform) + } + }) + } +} From f453a3a2f0c077cd5de7d20b2e703cc60b1a4704 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 26 Mar 2026 17:24:23 +0100 Subject: [PATCH 3/7] go.mod: bump minimum go version to go1.24 Signed-off-by: Sebastiaan van Stijn --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 50c854c..7effaf8 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/containerd/platforms -go 1.21 +go 1.24 require ( github.com/containerd/log v0.1.0 From 01651306b81698ca45b6196e86aa0b82292332eb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 26 Mar 2026 16:09:23 +0100 Subject: [PATCH 4/7] modernize --fix Signed-off-by: Sebastiaan van Stijn --- platforms.go | 11 ++++++----- platforms_test.go | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/platforms.go b/platforms.go index 2d9b3c2..30ff63b 100644 --- a/platforms.go +++ b/platforms.go @@ -372,18 +372,19 @@ func FormatAll(platform specs.Platform) string { return "unknown" } - osOptions := encodeOSOption(platform.OSVersion) + var b strings.Builder + b.WriteString(encodeOSOption(platform.OSVersion)) features := platform.OSFeatures if !slices.IsSorted(features) { features = slices.Clone(features) slices.Sort(features) } for _, f := range features { - osOptions += "+" + encodeOSOption(f) + b.WriteString("+" + encodeOSOption(f)) } - if osOptions != "" { - OSAndVersion := fmt.Sprintf("%s(%s)", platform.OS, osOptions) - return path.Join(OSAndVersion, platform.Architecture, platform.Variant) + if b.Len() > 0 { + osAndVersion := platform.OS + "(" + b.String() + ")" + return path.Join(osAndVersion, platform.Architecture, platform.Variant) } return path.Join(platform.OS, platform.Architecture, platform.Variant) } diff --git a/platforms_test.go b/platforms_test.go index d41ec8e..e0fb385 100644 --- a/platforms_test.go +++ b/platforms_test.go @@ -633,7 +633,7 @@ func BenchmarkParseOSOptions(b *testing.B) { b.ResetTimer() for _, bm := range benchmarks { b.Run(bm.doc, func(b *testing.B) { - for i := 0; i < b.N; i++ { + for range b.N { _, _ = Parse(bm.input) } }) @@ -668,7 +668,7 @@ func BenchmarkFormatAllOSFeatures(b *testing.B) { platform: specs.Platform{ OS: "linux", OSFeatures: func() (out []string) { - for i := 0; i <= maxFeatures; i++ { + for range maxFeatures { out = append(out, "feature") } return out @@ -725,7 +725,7 @@ func BenchmarkFormatAllOSFeatures(b *testing.B) { b.ResetTimer() for _, bm := range benchmarks { b.Run(bm.doc, func(b *testing.B) { - for i := 0; i < b.N; i++ { + for range b.N { _ = FormatAll(bm.platform) } }) From 8f5e31ac5040d9f6b0a6b8d8754338f7c999236d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 26 Mar 2026 16:19:27 +0100 Subject: [PATCH 5/7] FormatAll: use a string-builder for formatting os-options Signed-off-by: Sebastiaan van Stijn --- platforms.go | 46 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/platforms.go b/platforms.go index 30ff63b..77e03c1 100644 --- a/platforms.go +++ b/platforms.go @@ -373,20 +373,50 @@ func FormatAll(platform specs.Platform) string { } var b strings.Builder - b.WriteString(encodeOSOption(platform.OSVersion)) - features := platform.OSFeatures + b.WriteString(platform.OS) + osv := encodeOSOption(platform.OSVersion) + formatted := formatOSFeatures(platform.OSFeatures) + if osv != "" || formatted != "" { + b.Grow(len(osv) + len(formatted) + 3) // parens + maybe '+' + b.WriteByte('(') + if osv != "" { + b.WriteString(osv) + } + if formatted != "" { + b.WriteByte('+') + b.WriteString(formatted) + } + b.WriteByte(')') + } + + return path.Join(b.String(), platform.Architecture, platform.Variant) +} + +func formatOSFeatures(features []string) string { + if len(features) == 0 { + return "" + } + if !slices.IsSorted(features) { features = slices.Clone(features) slices.Sort(features) } + var b strings.Builder + var wrote bool + var prev string for _, f := range features { - b.WriteString("+" + encodeOSOption(f)) - } - if b.Len() > 0 { - osAndVersion := platform.OS + "(" + b.String() + ")" - return path.Join(osAndVersion, platform.Architecture, platform.Variant) + if f == "" || f == prev { + // skip empty and duplicate values + continue + } + prev = f + if wrote { + b.WriteByte('+') + } + b.WriteString(encodeOSOption(f)) + wrote = true } - return path.Join(platform.OS, platform.Architecture, platform.Variant) + return b.String() } // osOptionReplacer encodes characters in OS option values (version and From d028ee3f77b032d08f03b20412e6ea27b26dbb04 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 26 Mar 2026 16:45:40 +0100 Subject: [PATCH 6/7] ParseAll: refactor Avoid using `strings.Split`, and error on empty values. Signed-off-by: Sebastiaan van Stijn --- platforms.go | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/platforms.go b/platforms.go index 77e03c1..e322acf 100644 --- a/platforms.go +++ b/platforms.go @@ -280,13 +280,9 @@ func Parse(specifier string) (specs.Platform, error) { } p.OSVersion = osVersion if osOptions[3] != "" { - 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) - } + p.OSFeatures, err = parseOSFeatures(osOptions[3][1:]) + if err != nil { + return specs.Platform{}, fmt.Errorf("%q has invalid OS features: %w", specifier, err) } } } else { @@ -346,6 +342,30 @@ func Parse(specifier string) (specs.Platform, error) { return specs.Platform{}, fmt.Errorf("%q: cannot parse platform specifier: %w", specifier, errInvalidArgument) } +func parseOSFeatures(s string) ([]string, error) { + if s == "" { + return nil, nil + } + + var features []string + for raw := range strings.SplitSeq(s, "+") { + raw = strings.TrimSpace(raw) + if raw == "" { + return nil, fmt.Errorf("empty os feature: %w", errInvalidArgument) + } + feature, err := decodeOSOption(raw) + if err != nil { + return nil, fmt.Errorf("invalid os feature %q: %w", raw, err) + } + if feature == "" { + continue + } + features = append(features, feature) + } + + return features, nil +} + // MustParse is like Parses but panics if the specifier cannot be parsed. // Simplifies initialization of global variables. func MustParse(specifier string) specs.Platform { @@ -371,6 +391,9 @@ func FormatAll(platform specs.Platform) string { if platform.OS == "" { return "unknown" } + if platform.OSVersion == "" && len(platform.OSFeatures) == 0 { + return path.Join(platform.OS, platform.Architecture, platform.Variant) + } var b strings.Builder b.WriteString(platform.OS) From eb1ec17d98a9457d8435ad70924596dd26ba5378 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 26 Mar 2026 17:09:44 +0100 Subject: [PATCH 7/7] Add some limits to os-features Signed-off-by: Sebastiaan van Stijn --- platforms.go | 34 +++++++++++++++++++++++++++++++--- platforms_test.go | 4 ---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/platforms.go b/platforms.go index e322acf..76f8d2e 100644 --- a/platforms.go +++ b/platforms.go @@ -127,6 +127,12 @@ var ( osRe = regexp.MustCompile(`^([A-Za-z0-9_-]+)(?:\(([A-Za-z0-9_.%-]*)((?:\+[A-Za-z0-9_.%-]+)*)\))?$`) ) +const ( + maxFeatures = 16 + maxFeatureLen = 64 + maxOSOptionsLen = 256 +) + // Platform is a type alias for convenience, so there is no need to import image-spec package everywhere. type Platform = specs.Platform @@ -346,13 +352,22 @@ func parseOSFeatures(s string) ([]string, error) { if s == "" { return nil, nil } + if len(s) > maxOSOptionsLen { + return nil, fmt.Errorf("os features too long: %w", errInvalidArgument) + } - var features []string + features := make([]string, 0, min(strings.Count(s, "+")+1, maxFeatures)) for raw := range strings.SplitSeq(s, "+") { raw = strings.TrimSpace(raw) if raw == "" { return nil, fmt.Errorf("empty os feature: %w", errInvalidArgument) } + if len(features) == maxFeatures { + return nil, fmt.Errorf("too many os features: %w", errInvalidArgument) + } + if len(raw) > maxFeatureLen { + return nil, fmt.Errorf("os feature too long: %w", errInvalidArgument) + } feature, err := decodeOSOption(raw) if err != nil { return nil, fmt.Errorf("invalid os feature %q: %w", raw, err) @@ -416,7 +431,7 @@ func FormatAll(platform specs.Platform) string { } func formatOSFeatures(features []string) string { - if len(features) == 0 { + if len(features) == 0 || len(features) > maxFeatures { return "" } @@ -428,15 +443,28 @@ func formatOSFeatures(features []string) string { var wrote bool var prev string for _, f := range features { + if len(f) > maxFeatureLen { + // invalid + return "" + } if f == "" || f == prev { // skip empty and duplicate values continue } prev = f + + encoded := encodeOSOption(f) + if b.Len()+len(encoded) > maxOSOptionsLen { + return "" + } + if wrote { + if b.Len()+1 > maxOSOptionsLen { + return "" + } b.WriteByte('+') } - b.WriteString(encodeOSOption(f)) + b.WriteString(encoded) wrote = true } return b.String() diff --git a/platforms_test.go b/platforms_test.go index e0fb385..ea3fd3d 100644 --- a/platforms_test.go +++ b/platforms_test.go @@ -605,8 +605,6 @@ func FuzzPlatformsParse(f *testing.F) { } func BenchmarkParseOSOptions(b *testing.B) { - maxFeatures := 16 - benchmarks := []struct { doc string input string @@ -641,8 +639,6 @@ func BenchmarkParseOSOptions(b *testing.B) { } func BenchmarkFormatAllOSFeatures(b *testing.B) { - maxFeatures := 16 - benchmarks := []struct { doc string platform specs.Platform