From b300989ca3c9c8581d5b3457e6584cdf89ec0e23 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 30 Sep 2024 14:02:38 +0200 Subject: [PATCH 1/2] frontend/dockerfile: BFlags.Parse: return earlier on "--" terminator Minor optimization; put the check for a literal "--" first, instead of matching for "--" as prefix. Also add some documentation to the code, to outline why we return early. Signed-off-by: Sebastiaan van Stijn --- frontend/dockerfile/instructions/bflag.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/frontend/dockerfile/instructions/bflag.go b/frontend/dockerfile/instructions/bflag.go index 66e50d8aad2f..42f2fdcc50ee 100644 --- a/frontend/dockerfile/instructions/bflag.go +++ b/frontend/dockerfile/instructions/bflag.go @@ -148,14 +148,21 @@ func (bf *BFlags) Parse() error { } for _, arg := range bf.Args { - if !strings.HasPrefix(arg, "--") { - return errors.Errorf("arg should start with -- : %s", arg) - } - if arg == "--" { + // Stop processing further arguments as flags. We're matching + // the POSIX Utility Syntax Guidelines here; + // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02 + // + // > The first -- argument that is not an option-argument should be accepted + // > as a delimiter indicating the end of options. Any following arguments + // > should be treated as operands, even if they begin with the '-' character. return nil } + if !strings.HasPrefix(arg, "--") { + return errors.Errorf("arg should start with -- : %s", arg) + } + arg, value, hasValue := strings.Cut(arg[2:], "=") flag, ok := bf.flags[arg] From 31a7c177e96f97f619d45d90aa37a8073d96fcae Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 30 Sep 2024 14:33:02 +0200 Subject: [PATCH 2/2] frontend/dockerfile: BFlags.Parse: include flag with "--" prefix in errors Before this change, only the name of the flag would be printed as part of errors. Change the errors to include the flag-name _including_ the "--" prefix to make it more clear the error is about a flag that was provided but invalid. Before this change: Dockerfile:2 -------------------- 1 | FROM alpine 2 | >>> COPY --nosuchflag . . 3 | -------------------- ERROR: failed to solve: dockerfile parse error on line 2: unknown flag: nosuchflag Dockerfile:4 -------------------- 2 | FROM alpine 3 | WORKDIR /test 4 | >>> COPY --exclude /.git . . 5 | -------------------- ERROR: failed to solve: dockerfile parse error on line 4: missing a value on flag: exclude With this change; Dockerfile:2 -------------------- 1 | FROM alpine 2 | >>> COPY --nosuchflag . . 3 | -------------------- ERROR: failed to solve: dockerfile parse error on line 2: unknown flag: --nosuchflag Dockerfile:4 -------------------- 2 | FROM alpine 3 | WORKDIR /test 4 | >>> COPY --exclude /.git . . 5 | -------------------- ERROR: failed to solve: dockerfile parse error on line 4: missing a value on flag: --exclude Signed-off-by: Sebastiaan van Stijn --- frontend/dockerfile/dockerfile_mount_test.go | 2 +- frontend/dockerfile/instructions/bflag.go | 24 +++++++++---------- .../dockerfile/instructions/parse_test.go | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/frontend/dockerfile/dockerfile_mount_test.go b/frontend/dockerfile/dockerfile_mount_test.go index 0f408cc15c0a..56e4031b2595 100644 --- a/frontend/dockerfile/dockerfile_mount_test.go +++ b/frontend/dockerfile/dockerfile_mount_test.go @@ -115,7 +115,7 @@ RUN --mont=target=/mytmp,type=tmpfs /bin/true }, }, nil) require.Error(t, err) - require.Contains(t, err.Error(), "unknown flag: mont") + require.Contains(t, err.Error(), "unknown flag: --mont") require.Contains(t, err.Error(), "did you mean mount?") dockerfile = []byte(` diff --git a/frontend/dockerfile/instructions/bflag.go b/frontend/dockerfile/instructions/bflag.go index 42f2fdcc50ee..a67778ea746a 100644 --- a/frontend/dockerfile/instructions/bflag.go +++ b/frontend/dockerfile/instructions/bflag.go @@ -147,8 +147,8 @@ func (bf *BFlags) Parse() error { return errors.Wrap(bf.Err, "error setting up flags") } - for _, arg := range bf.Args { - if arg == "--" { + for _, a := range bf.Args { + if a == "--" { // Stop processing further arguments as flags. We're matching // the POSIX Utility Syntax Guidelines here; // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02 @@ -158,21 +158,21 @@ func (bf *BFlags) Parse() error { // > should be treated as operands, even if they begin with the '-' character. return nil } - - if !strings.HasPrefix(arg, "--") { - return errors.Errorf("arg should start with -- : %s", arg) + if !strings.HasPrefix(a, "--") { + return errors.Errorf("arg should start with -- : %s", a) } - arg, value, hasValue := strings.Cut(arg[2:], "=") + flagName, value, hasValue := strings.Cut(a, "=") + arg := flagName[2:] flag, ok := bf.flags[arg] if !ok { - err := errors.Errorf("unknown flag: %s", arg) + err := errors.Errorf("unknown flag: %s", flagName) return suggest.WrapError(err, arg, allFlags(bf.flags), true) } if _, ok = bf.used[arg]; ok && flag.flagType != stringsType { - return errors.Errorf("duplicate flag specified: %s", arg) + return errors.Errorf("duplicate flag specified: %s", flagName) } bf.used[arg] = flag @@ -181,7 +181,7 @@ func (bf *BFlags) Parse() error { case boolType: // value == "" is only ok if no "=" was specified if hasValue && value == "" { - return errors.Errorf("missing a value on flag: %s", arg) + return errors.Errorf("missing a value on flag: %s", flagName) } switch strings.ToLower(value) { @@ -190,18 +190,18 @@ func (bf *BFlags) Parse() error { case "false": flag.Value = "false" default: - return errors.Errorf("expecting boolean value for flag %s, not: %s", arg, value) + return errors.Errorf("expecting boolean value for flag %s, not: %s", flagName, value) } case stringType: if !hasValue { - return errors.Errorf("missing a value on flag: %s", arg) + return errors.Errorf("missing a value on flag: %s", flagName) } flag.Value = value case stringsType: if !hasValue { - return errors.Errorf("missing a value on flag: %s", arg) + return errors.Errorf("missing a value on flag: %s", flagName) } flag.StringValues = append(flag.StringValues, value) diff --git a/frontend/dockerfile/instructions/parse_test.go b/frontend/dockerfile/instructions/parse_test.go index 111820a5f024..e1adaa6d73f4 100644 --- a/frontend/dockerfile/instructions/parse_test.go +++ b/frontend/dockerfile/instructions/parse_test.go @@ -229,7 +229,7 @@ func TestErrorCases(t *testing.T) { { name: "MAINTAINER unknown flag", dockerfile: "MAINTAINER --boo joe@example.com", - expectedError: "unknown flag: boo", + expectedError: "unknown flag: --boo", }, { name: "Chaining ONBUILD",