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/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 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) { } }) } - } diff --git a/platforms.go b/platforms.go index 2d9b3c2..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 @@ -280,13 +286,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 +348,39 @@ 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 + } + if len(s) > maxOSOptionsLen { + return nil, fmt.Errorf("os features too long: %w", errInvalidArgument) + } + + 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) + } + 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,21 +406,68 @@ 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) + 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 || len(features) > maxFeatures { + return "" + } - osOptions := encodeOSOption(platform.OSVersion) - features := platform.OSFeatures 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 { - osOptions += "+" + encodeOSOption(f) - } - if osOptions != "" { - OSAndVersion := fmt.Sprintf("%s(%s)", platform.OS, osOptions) - return path.Join(OSAndVersion, platform.Architecture, platform.Variant) + 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(encoded) + wrote = true } - return path.Join(platform.OS, platform.Architecture, platform.Variant) + return b.String() } // osOptionReplacer encodes characters in OS option values (version and diff --git a/platforms_test.go b/platforms_test.go index dddecb8..ea3fd3d 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,127 @@ func FuzzPlatformsParse(f *testing.F) { } }) } + +func BenchmarkParseOSOptions(b *testing.B) { + 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 range b.N { + _, _ = Parse(bm.input) + } + }) + } +} + +func BenchmarkFormatAllOSFeatures(b *testing.B) { + 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 range maxFeatures { + 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 range b.N { + _ = FormatAll(bm.platform) + } + }) + } +}