From 90a62d7b0c1620a167002eb10a5e2f13924272de Mon Sep 17 00:00:00 2001 From: Roberto Hidalgo Date: Mon, 25 Nov 2019 14:25:11 -0500 Subject: [PATCH 1/5] Prevent panic on flagSet access from custom BashComplete Fixes #944 --- command.go | 7 ++++--- command_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/command.go b/command.go index a579fdb06a..a99da0a1b9 100644 --- a/command.go +++ b/command.go @@ -100,7 +100,7 @@ func (c *Command) Run(ctx *Context) (err error) { c.UseShortOptionHandling = true } - set, err := c.parseFlags(ctx.Args()) + set, err := c.parseFlags(ctx.Args(), ctx.shellComplete) context := NewContext(ctx.App, set, ctx) context.Command = c @@ -174,7 +174,7 @@ func (c *Command) useShortOptionHandling() bool { return c.UseShortOptionHandling } -func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) { +func (c *Command) parseFlags(args Args, shellComplete bool) (*flag.FlagSet, error) { set, err := c.newFlagSet() if err != nil { return nil, err @@ -185,7 +185,8 @@ func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) { } err = parseIter(set, c, args.Tail()) - if err != nil { + // Continue parsing flags on failure during shell completion + if err != nil && !shellComplete { return nil, err } diff --git a/command_test.go b/command_test.go index 7c55f138da..8eafe68b48 100644 --- a/command_test.go +++ b/command_test.go @@ -1,6 +1,7 @@ package cli import ( + "bytes" "errors" "flag" "fmt" @@ -329,3 +330,49 @@ func TestCommandSkipFlagParsing(t *testing.T) { expect(t, args, c.expectedArgs) } } + +func TestCommand_Run_CustomShellCompleteAcceptsMalformedFlags(t *testing.T) { + cases := []struct { + testArgs args + expectedOut string + }{ + {testArgs: args{"--undefined"}, expectedOut: "found 0 args"}, + {testArgs: args{"--number"}, expectedOut: "found 0 args"}, + {testArgs: args{"--number", "fourty-two"}, expectedOut: "found 0 args"}, + {testArgs: args{"--number", "42"}, expectedOut: "found 0 args"}, + {testArgs: args{"--number", "42", "newArg"}, expectedOut: "found 1 args"}, + } + + for _, c := range cases { + var outputBuffer bytes.Buffer + app := &App{ + Writer: &outputBuffer, + EnableBashCompletion: true, + Commands: []*Command{ + { + Name: "bar", + Usage: "this is for testing", + Flags: []Flag{ + &IntFlag{ + Name: "number", + Usage: "A number to parse", + }, + }, + BashComplete: func(c *Context) { + fmt.Fprintf(c.App.Writer, "found %d args", c.NArg()) + }, + }, + }, + } + + osArgs := args{"foo", "bar"} + osArgs = append(osArgs, c.testArgs...) + osArgs = append(osArgs, "--generate-bash-completion") + + err := app.Run(osArgs) + stdout := outputBuffer.String() + expect(t, err, nil) + expect(t, stdout, c.expectedOut) + } + +} From f3295e3cdb6bfe573aca0b01af8ddd9335226264 Mon Sep 17 00:00:00 2001 From: Roberto Hidalgo Date: Wed, 27 Nov 2019 11:42:48 -0500 Subject: [PATCH 2/5] Check for parsing errors within parse.go:parseIter Add description to that function's docstring, and delete extraneous space --- app.go | 4 ++-- command.go | 5 ++--- parse.go | 11 ++++++++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app.go b/app.go index d03ec5d71d..16b0b19c9e 100644 --- a/app.go +++ b/app.go @@ -223,7 +223,7 @@ func (a *App) Run(arguments []string) (err error) { return err } - err = parseIter(set, a, arguments[1:]) + err = parseIter(set, a, arguments[1:], shellComplete) nerr := normalizeFlags(a.Flags, set) context := NewContext(a, set, nil) if nerr != nil { @@ -349,7 +349,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { return err } - err = parseIter(set, a, ctx.Args().Tail()) + err = parseIter(set, a, ctx.Args().Tail(), ctx.shellComplete) nerr := normalizeFlags(a.Flags, set) context := NewContext(a, set, ctx) diff --git a/command.go b/command.go index a99da0a1b9..db6c8024ba 100644 --- a/command.go +++ b/command.go @@ -184,9 +184,8 @@ func (c *Command) parseFlags(args Args, shellComplete bool) (*flag.FlagSet, erro return set, set.Parse(append([]string{"--"}, args.Tail()...)) } - err = parseIter(set, c, args.Tail()) - // Continue parsing flags on failure during shell completion - if err != nil && !shellComplete { + err = parseIter(set, c, args.Tail(), shellComplete) + if err != nil { return nil, err } diff --git a/parse.go b/parse.go index 660f538b03..e77568d7dd 100644 --- a/parse.go +++ b/parse.go @@ -11,19 +11,24 @@ type iterativeParser interface { } // To enable short-option handling (e.g., "-it" vs "-i -t") we have to -// iteratively catch parsing errors. This way we achieve LR parsing without +// iteratively catch parsing errors. This way we achieve LR parsing without // transforming any arguments. Otherwise, there is no way we can discriminate // combined short options from common arguments that should be left untouched. -func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error { +// Pass `ignoreErrors` to continue parsing options on failure, for example +// during shell completion when the user-supplied options may be incomplete. +func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, ignoreErrors bool) error { for { err := set.Parse(args) if !ip.useShortOptionHandling() || err == nil { + if ignoreErrors { + return nil + } return err } errStr := err.Error() trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: -") - if errStr == trimmed { + if !ignoreErrors && errStr == trimmed { return err } From 9e7226ec28b02c14aab550c2028c33acb3439fa9 Mon Sep 17 00:00:00 2001 From: Roberto Hidalgo Date: Wed, 27 Nov 2019 11:45:44 -0500 Subject: [PATCH 3/5] use `shellComplete instead of `ignoreErrors`, clean up --- parse.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/parse.go b/parse.go index e77568d7dd..7df17296a4 100644 --- a/parse.go +++ b/parse.go @@ -14,13 +14,13 @@ type iterativeParser interface { // iteratively catch parsing errors. This way we achieve LR parsing without // transforming any arguments. Otherwise, there is no way we can discriminate // combined short options from common arguments that should be left untouched. -// Pass `ignoreErrors` to continue parsing options on failure, for example -// during shell completion when the user-supplied options may be incomplete. -func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, ignoreErrors bool) error { +// Pass `shellComplete` to continue parsing options on failure during shell +// completion when, the user-supplied options may be incomplete. +func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, shellComplete bool) error { for { err := set.Parse(args) if !ip.useShortOptionHandling() || err == nil { - if ignoreErrors { + if shellComplete { return nil } return err @@ -28,7 +28,7 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, ignoreError errStr := err.Error() trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: -") - if !ignoreErrors && errStr == trimmed { + if errStr == trimmed { return err } From d733c547011bb6fff5be1d3fe19d5bfb5fab26ef Mon Sep 17 00:00:00 2001 From: "lynn [they]" Date: Wed, 27 Nov 2019 20:36:07 -0800 Subject: [PATCH 4/5] testing the tests --- parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse.go b/parse.go index 7df17296a4..a365d8260f 100644 --- a/parse.go +++ b/parse.go @@ -21,7 +21,7 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, shellComple err := set.Parse(args) if !ip.useShortOptionHandling() || err == nil { if shellComplete { - return nil + return err } return err } From 233958cd0047a60fd945f381b9142ec5b58b1f8f Mon Sep 17 00:00:00 2001 From: "lynn [they]" Date: Wed, 27 Nov 2019 20:37:23 -0800 Subject: [PATCH 5/5] revert testing change --- parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse.go b/parse.go index a365d8260f..7df17296a4 100644 --- a/parse.go +++ b/parse.go @@ -21,7 +21,7 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, shellComple err := set.Parse(args) if !ip.useShortOptionHandling() || err == nil { if shellComplete { - return err + return nil } return err }