From df740f656281131043563a9a3b92f83a2e495fa7 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Fri, 11 Oct 2019 22:32:21 -0400 Subject: [PATCH 1/2] Fix infinite loop flag parsing with short options --- command_test.go | 5 +++++ parse.go | 17 ++++++++--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/command_test.go b/command_test.go index 6235b41db5..66859c07d8 100644 --- a/command_test.go +++ b/command_test.go @@ -70,8 +70,13 @@ func TestParseAndRunShortOpts(t *testing.T) { {[]string{"foo", "test", "-af"}, nil, []string{}}, {[]string{"foo", "test", "-cf"}, nil, []string{}}, {[]string{"foo", "test", "-acf"}, nil, []string{}}, + {[]string{"foo", "test", "--acf"}, errors.New("flag provided but not defined: -acf"), nil}, {[]string{"foo", "test", "-invalid"}, errors.New("flag provided but not defined: -invalid"), nil}, + {[]string{"foo", "test", "-acf", "-invalid"}, errors.New("flag provided but not defined: -invalid"), nil}, + {[]string{"foo", "test", "--invalid"}, errors.New("flag provided but not defined: -invalid"), nil}, + {[]string{"foo", "test", "-acf", "--invalid"}, errors.New("flag provided but not defined: -invalid"), nil}, {[]string{"foo", "test", "-acf", "arg1", "-invalid"}, nil, []string{"arg1", "-invalid"}}, + {[]string{"foo", "test", "-acf", "arg1", "--invalid"}, nil, []string{"arg1", "--invalid"}}, {[]string{"foo", "test", "-acfi", "not-arg", "arg1", "-invalid"}, nil, []string{"arg1", "-invalid"}}, {[]string{"foo", "test", "-i", "ivalue"}, nil, []string{}}, {[]string{"foo", "test", "-i", "ivalue", "arg1"}, nil, []string{"arg1"}}, diff --git a/parse.go b/parse.go index 2c2005c13a..cab99e504f 100644 --- a/parse.go +++ b/parse.go @@ -22,28 +22,27 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error { } errStr := err.Error() - trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: ") + trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: -") if errStr == trimmed { return err } // regenerate the initial args with the split short opts - newArgs := []string{} for i, arg := range args { - if arg != trimmed { - newArgs = append(newArgs, arg) + // skip args that are not part of the error message + if name := strings.TrimLeft(arg, "-"); name != trimmed { continue } - shortOpts := splitShortOptions(set, trimmed) + // if we can't split, the error was accurate + shortOpts := splitShortOptions(set, arg) if len(shortOpts) == 1 { return err } - // add each short option and all remaining arguments - newArgs = append(newArgs, shortOpts...) - newArgs = append(newArgs, args[i+1:]...) - args = newArgs + // swap current argument with the split version + args = append(args[:i], append(shortOpts, args[i+1:]...)...) + break } // Since custom parsing failed, replace the flag set before retrying From f5306b622f216cafde2a670b9a9e620cb6b0fad1 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Fri, 11 Oct 2019 22:57:55 -0400 Subject: [PATCH 2/2] Ensure infinite loop cannot occur in parsing --- parse.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/parse.go b/parse.go index cab99e504f..660f538b03 100644 --- a/parse.go +++ b/parse.go @@ -28,6 +28,7 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error { } // regenerate the initial args with the split short opts + argsWereSplit := false for i, arg := range args { // skip args that are not part of the error message if name := strings.TrimLeft(arg, "-"); name != trimmed { @@ -42,9 +43,16 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error { // swap current argument with the split version args = append(args[:i], append(shortOpts, args[i+1:]...)...) + argsWereSplit = true break } + // This should be an impossible to reach code path, but in case the arg + // splitting failed to happen, this will prevent infinite loops + if !argsWereSplit { + return err + } + // Since custom parsing failed, replace the flag set before retrying newSet, err := ip.newFlagSet() if err != nil {