Skip to content

Commit e543b9f

Browse files
samuelkarpsamuelkarp
authored
Merge pull request #28 from dmcgowan/encode-os-version
Add encoding to os version and features
2 parents 54c1ef4 + 3cff7fa commit e543b9f

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.
@@ -273,9 +274,20 @@ func Parse(specifier string) (specs.Platform, error) {
273274
}
274275

275276
p.OS = normalizeOS(osOptions[1])
276-
p.OSVersion = osOptions[2]
277+
osVersion, err := decodeOSOption(osOptions[2])
278+
if err != nil {
279+
return specs.Platform{}, fmt.Errorf("%q has an invalid OS version %q: %w", specifier, osOptions[2], err)
280+
}
281+
p.OSVersion = osVersion
277282
if osOptions[3] != "" {
278-
p.OSFeatures = strings.Split(osOptions[3][1:], "+")
283+
rawFeatures := strings.Split(osOptions[3][1:], "+")
284+
p.OSFeatures = make([]string, len(rawFeatures))
285+
for i, f := range rawFeatures {
286+
p.OSFeatures[i], err = decodeOSOption(f)
287+
if err != nil {
288+
return specs.Platform{}, fmt.Errorf("%q has an invalid OS feature %q: %w", specifier, f, err)
289+
}
290+
}
279291
}
280292
} else {
281293
if !specifierRe.MatchString(part) {
@@ -360,14 +372,14 @@ func FormatAll(platform specs.Platform) string {
360372
return "unknown"
361373
}
362374

363-
osOptions := platform.OSVersion
375+
osOptions := encodeOSOption(platform.OSVersion)
364376
features := platform.OSFeatures
365377
if !slices.IsSorted(features) {
366378
features = slices.Clone(features)
367379
slices.Sort(features)
368380
}
369-
if len(features) > 0 {
370-
osOptions += "+" + strings.Join(features, "+")
381+
for _, f := range features {
382+
osOptions += "+" + encodeOSOption(f)
371383
}
372384
if osOptions != "" {
373385
OSAndVersion := fmt.Sprintf("%s(%s)", platform.OS, osOptions)
@@ -376,6 +388,28 @@ func FormatAll(platform specs.Platform) string {
376388
return path.Join(platform.OS, platform.Architecture, platform.Variant)
377389
}
378390

391+
// osOptionReplacer encodes characters in OS option values (version and
392+
// features) that are ambiguous with the format syntax. The percent sign
393+
// must be replaced first to avoid double-encoding.
394+
var osOptionReplacer = strings.NewReplacer(
395+
"%", "%25",
396+
"+", "%2B",
397+
"(", "%28",
398+
")", "%29",
399+
"/", "%2F",
400+
)
401+
402+
func encodeOSOption(v string) string {
403+
return osOptionReplacer.Replace(v)
404+
}
405+
406+
func decodeOSOption(v string) (string, error) {
407+
if strings.Contains(v, "%") {
408+
return url.PathUnescape(v)
409+
}
410+
return v, nil
411+
}
412+
379413
// Normalize validates and translate the platform to the canonical value.
380414
//
381415
// 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)