Skip to content

Commit 3cff7fa

Browse files
dmcgowandmcgowan
committed
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 <[email protected]>
1 parent b42036f commit 3cff7fa

2 files changed

Lines changed: 141 additions & 6 deletions

File tree

‎platforms.go‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ package platforms
111111

112112
import (
113113
"fmt"
114+
"net/url"
114115
"path"
115116
"regexp"
116117
"runtime"
@@ -123,7 +124,7 @@ import (
123124

124125
var (
125126
specifierRe = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`)
126-
osRe = regexp.MustCompile(`^([A-Za-z0-9_-]+)(?:\(([A-Za-z0-9_.-]*)((?:\+[A-Za-z0-9_.-]+)*)\))?$`)
127+
osRe = regexp.MustCompile(`^([A-Za-z0-9_-]+)(?:\(([A-Za-z0-9_.%-]*)((?:\+[A-Za-z0-9_.%-]+)*)\))?$`)
127128
)
128129

129130
// 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) {
240241
}
241242

242243
p.OS = normalizeOS(osOptions[1])
243-
p.OSVersion = osOptions[2]
244+
osVersion, err := decodeOSOption(osOptions[2])
245+
if err != nil {
246+
return specs.Platform{}, fmt.Errorf("%q has an invalid OS version %q: %w", specifier, osOptions[2], err)
247+
}
248+
p.OSVersion = osVersion
244249
if osOptions[3] != "" {
245-
p.OSFeatures = strings.Split(osOptions[3][1:], "+")
250+
rawFeatures := strings.Split(osOptions[3][1:], "+")
251+
p.OSFeatures = make([]string, len(rawFeatures))
252+
for i, f := range rawFeatures {
253+
p.OSFeatures[i], err = decodeOSOption(f)
254+
if err != nil {
255+
return specs.Platform{}, fmt.Errorf("%q has an invalid OS feature %q: %w", specifier, f, err)
256+
}
257+
}
246258
}
247259
} else {
248260
if !specifierRe.MatchString(part) {
@@ -327,14 +339,14 @@ func FormatAll(platform specs.Platform) string {
327339
return "unknown"
328340
}
329341

330-
osOptions := platform.OSVersion
342+
osOptions := encodeOSOption(platform.OSVersion)
331343
features := platform.OSFeatures
332344
if !slices.IsSorted(features) {
333345
features = slices.Clone(features)
334346
slices.Sort(features)
335347
}
336-
if len(features) > 0 {
337-
osOptions += "+" + strings.Join(features, "+")
348+
for _, f := range features {
349+
osOptions += "+" + encodeOSOption(f)
338350
}
339351
if osOptions != "" {
340352
OSAndVersion := fmt.Sprintf("%s(%s)", platform.OS, osOptions)
@@ -343,6 +355,28 @@ func FormatAll(platform specs.Platform) string {
343355
return path.Join(platform.OS, platform.Architecture, platform.Variant)
344356
}
345357

358+
// osOptionReplacer encodes characters in OS option values (version and
359+
// features) that are ambiguous with the format syntax. The percent sign
360+
// must be replaced first to avoid double-encoding.
361+
var osOptionReplacer = strings.NewReplacer(
362+
"%", "%25",
363+
"+", "%2B",
364+
"(", "%28",
365+
")", "%29",
366+
"/", "%2F",
367+
)
368+
369+
func encodeOSOption(v string) string {
370+
return osOptionReplacer.Replace(v)
371+
}
372+
373+
func decodeOSOption(v string) (string, error) {
374+
if strings.Contains(v, "%") {
375+
return url.PathUnescape(v)
376+
}
377+
return v, nil
378+
}
379+
346380
// Normalize validates and translate the platform to the canonical value.
347381
//
348382
// For example, if "Aarch64" is encountered, we change it to "arm64" or if

‎platforms_test.go‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,40 @@ func TestParseSelector(t *testing.T) {
391391
formatted: path.Join("linux(+erofs+unsorted)", defaultArch, defaultVariant),
392392
useV2Format: true,
393393
},
394+
{
395+
input: "windows(10.0.17763%2Bbuild.42)",
396+
expected: specs.Platform{
397+
OS: "windows",
398+
OSVersion: "10.0.17763+build.42",
399+
Architecture: defaultArch,
400+
Variant: defaultVariant,
401+
},
402+
formatted: path.Join("windows(10.0.17763%2Bbuild.42)", defaultArch, defaultVariant),
403+
useV2Format: true,
404+
},
405+
{
406+
input: "windows(10.0.17763%2Bbuild.42+win32k)",
407+
expected: specs.Platform{
408+
OS: "windows",
409+
OSVersion: "10.0.17763+build.42",
410+
OSFeatures: []string{"win32k"},
411+
Architecture: defaultArch,
412+
Variant: defaultVariant,
413+
},
414+
formatted: path.Join("windows(10.0.17763%2Bbuild.42+win32k)", defaultArch, defaultVariant),
415+
useV2Format: true,
416+
},
417+
{
418+
input: "windows(50%25done)",
419+
expected: specs.Platform{
420+
OS: "windows",
421+
OSVersion: "50%done",
422+
Architecture: defaultArch,
423+
Variant: defaultVariant,
424+
},
425+
formatted: path.Join("windows(50%25done)", defaultArch, defaultVariant),
426+
useV2Format: true,
427+
},
394428
} {
395429
t.Run(testcase.input, func(t *testing.T) {
396430
if testcase.skip {
@@ -446,6 +480,73 @@ func TestParseSelector(t *testing.T) {
446480
}
447481
}
448482

483+
func TestFormatAllEncoding(t *testing.T) {
484+
for _, testcase := range []struct {
485+
platform specs.Platform
486+
expected string
487+
}{
488+
{
489+
platform: specs.Platform{OS: "windows", OSVersion: "10.0.17763+build.42", Architecture: "amd64"},
490+
expected: "windows(10.0.17763%2Bbuild.42)/amd64",
491+
},
492+
{
493+
platform: specs.Platform{OS: "windows", OSVersion: "10.0.17763+build.42", OSFeatures: []string{"win32k"}, Architecture: "amd64"},
494+
expected: "windows(10.0.17763%2Bbuild.42+win32k)/amd64",
495+
},
496+
{
497+
platform: specs.Platform{OS: "windows", OSVersion: "50%done", Architecture: "amd64"},
498+
expected: "windows(50%25done)/amd64",
499+
},
500+
{
501+
platform: specs.Platform{OS: "windows", OSVersion: "1.0(beta)", Architecture: "amd64"},
502+
expected: "windows(1.0%28beta%29)/amd64",
503+
},
504+
{
505+
platform: specs.Platform{OS: "windows", OSVersion: "a/b", Architecture: "amd64"},
506+
expected: "windows(a%2Fb)/amd64",
507+
},
508+
{
509+
// no special characters, no encoding needed
510+
platform: specs.Platform{OS: "windows", OSVersion: "10.0.17763", Architecture: "amd64"},
511+
expected: "windows(10.0.17763)/amd64",
512+
},
513+
{
514+
// feature with + in the name
515+
platform: specs.Platform{OS: "linux", OSFeatures: []string{"feat+v2"}, Architecture: "amd64"},
516+
expected: "linux(+feat%2Bv2)/amd64",
517+
},
518+
{
519+
// feature with % in the name
520+
platform: specs.Platform{OS: "linux", OSFeatures: []string{"100%gpu"}, Architecture: "amd64"},
521+
expected: "linux(+100%25gpu)/amd64",
522+
},
523+
{
524+
// version and feature both with special characters
525+
platform: specs.Platform{OS: "windows", OSVersion: "10.0+build", OSFeatures: []string{"feat+1"}, Architecture: "amd64"},
526+
expected: "windows(10.0%2Bbuild+feat%2B1)/amd64",
527+
},
528+
} {
529+
t.Run(testcase.expected, func(t *testing.T) {
530+
formatted := FormatAll(testcase.platform)
531+
if formatted != testcase.expected {
532+
t.Fatalf("unexpected format: %q != %q", formatted, testcase.expected)
533+
}
534+
535+
// verify round-trip
536+
reparsed, err := Parse(formatted)
537+
if err != nil {
538+
t.Fatalf("error parsing formatted output: %v", err)
539+
}
540+
if reparsed.OSVersion != testcase.platform.OSVersion {
541+
t.Fatalf("OSVersion did not survive round trip: %q != %q", reparsed.OSVersion, testcase.platform.OSVersion)
542+
}
543+
if !reflect.DeepEqual(reparsed.OSFeatures, testcase.platform.OSFeatures) {
544+
t.Fatalf("OSFeatures did not survive round trip: %v != %v", reparsed.OSFeatures, testcase.platform.OSFeatures)
545+
}
546+
})
547+
}
548+
}
549+
449550
func TestParseSelectorInvalid(t *testing.T) {
450551
for _, testcase := range []struct {
451552
input string

0 commit comments

Comments
 (0)