Skip to content

Commit a942ac7

Browse files
committed
Review suggestions
- Split regex for first (os and version) and further components (arch, variant) - Update regular expression to preserve the existing set of accepted characters for the OS part of the first element (avoid accepting ".", ")", "("). - Make regular expression for osVersion more strict, and only accept dot-separated numbers. - Add capture groups to the osAndVersion regular expression so that the result can be consumed directly, without having to split after validating. - Now that we already separate OS asnd OSVersion ahead; simplify normalizeOS to only normalize the OS. - Remove the OSAndVersionFormat variable, and inline it. - Update Parse() to prevent un-bounded string splitting. Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent c1438e9 commit a942ac7

File tree

3 files changed

+44
-11
lines changed

3 files changed

+44
-11
lines changed

database.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ func isKnownArch(arch string) bool {
5959
return false
6060
}
6161

62+
// normalizeOS returns the normalized OS. If the os is empty, it returns
63+
// [runtime.GOOS].
6264
func normalizeOS(os string) string {
6365
if os == "" {
6466
return runtime.GOOS

platforms.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,11 @@
6565
// While the OCI platform specifications provide a tool for components to
6666
// specify structured information, user input typically doesn't need the full
6767
// context and much can be inferred. To solve this problem, we introduced
68-
// "specifiers". A specifier has the format
69-
// `<os>|<arch>|<os>/<arch>[/<variant>]`. The user can provide either the
70-
// operating system or the architecture or both.
68+
// "specifiers". A specifier has the format:
69+
//
70+
// <os>[(<osVersion>)]|<arch>|<os>/<arch>[/<variant>]
71+
//
72+
// The user can provide either the operating system or the architecture or both.
7173
//
7274
// An example of a common specifier is `linux/amd64`. If the host has a default
7375
// of runtime that matches this, the user can simply provide the component that

platforms_test.go

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,26 @@ func TestParseSelector(t *testing.T) {
343343
formatted: path.Join("windows(10.0.17763)", defaultArch, defaultVariant),
344344
useV2Format: true,
345345
},
346+
{
347+
input: "windows(10.0.17763)/amd64",
348+
expected: specs.Platform{
349+
OS: "windows",
350+
OSVersion: "10.0.17763",
351+
Architecture: "amd64",
352+
},
353+
formatted: "windows(10.0.17763)/amd64",
354+
useV2Format: true,
355+
},
356+
{
357+
input: "macos(Abcd.Efgh.123-4)/aarch64",
358+
expected: specs.Platform{
359+
OS: "darwin",
360+
OSVersion: "Abcd.Efgh.123-4",
361+
Architecture: "arm64",
362+
},
363+
formatted: "darwin(Abcd.Efgh.123-4)/arm64",
364+
useV2Format: true,
365+
},
346366
} {
347367
t.Run(testcase.input, func(t *testing.T) {
348368
if testcase.skip {
@@ -370,10 +390,10 @@ func TestParseSelector(t *testing.T) {
370390
}
371391

372392
formatted := ""
373-
if testcase.useV2Format == false {
374-
formatted = Format(p)
375-
} else {
393+
if testcase.useV2Format {
376394
formatted = FormatAll(p)
395+
} else {
396+
formatted = Format(p)
377397
}
378398
if formatted != testcase.formatted {
379399
t.Fatalf("unexpected format: %q != %q", formatted, testcase.formatted)
@@ -385,14 +405,14 @@ func TestParseSelector(t *testing.T) {
385405
t.Fatalf("error parsing formatted output: %v", err)
386406
}
387407

388-
if testcase.useV2Format == false {
389-
if Format(reparsed) != formatted {
390-
t.Fatalf("normalized output did not survive the round trip: %v != %v", Format(reparsed), formatted)
391-
}
392-
} else {
408+
if testcase.useV2Format {
393409
if FormatAll(reparsed) != formatted {
394410
t.Fatalf("normalized output did not survive the round trip: %v != %v", FormatAll(reparsed), formatted)
395411
}
412+
} else {
413+
if Format(reparsed) != formatted {
414+
t.Fatalf("normalized output did not survive the round trip: %v != %v", Format(reparsed), formatted)
415+
}
396416
}
397417
})
398418
}
@@ -420,6 +440,15 @@ func TestParseSelectorInvalid(t *testing.T) {
420440
{
421441
input: "linux/arm/foo/bar", // too many components
422442
},
443+
{
444+
input: "amd64/windows(10.0.17763)/foo", // only first element accepts os[(osVersion)]
445+
},
446+
{
447+
input: "linux)()---()..../arm/foo",
448+
},
449+
{
450+
input: "../arm/foo",
451+
},
423452
} {
424453
t.Run(testcase.input, func(t *testing.T) {
425454
if _, err := Parse(testcase.input); err == nil {

0 commit comments

Comments
 (0)