From 3bf635dc60b93a8178a3c0ca2d6cec36e5f15d37 Mon Sep 17 00:00:00 2001 From: Katy Moe Date: Thu, 21 Apr 2022 11:56:41 +0100 Subject: [PATCH 1/2] add failing test for subcommand argument parsing Add a failing test demonstrating that "fooasdf -o=foo" is incorrectly matched to the subcommand "foo". --- cli_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/cli_test.go b/cli_test.go index 3083937..e381d54 100644 --- a/cli_test.go +++ b/cli_test.go @@ -171,6 +171,40 @@ func TestCLIRun_prefix(t *testing.T) { } } +// NOTE: This failing test is intended to demonstrate incorrect behaviour +// in which a subcommand with an arbitrary suffix is still parsed as that +// subcommand, if a supplied argument is also suffixed by that subcommand. +func TestCLIRun_subcommandSuffix(t *testing.T) { + buf := new(bytes.Buffer) + command := new(MockCommand) + cli := &CLI{ + Args: []string{"fooasdf", "-o=foo"}, + Commands: map[string]CommandFactory{ + "foo": func() (Command, error) { + return command, nil + }, + + "foo bar": func() (Command, error) { + return command, nil + }, + }, + ErrorWriter: buf, + } + + exitCode, err := cli.Run() + if err != nil { + t.Fatalf("err: %s", err) + } + + if exitCode != 127 { + t.Fatalf("expected to get exit code 127, but got %d", exitCode) + } + + if command.RunCalled { + t.Fatalf("run should not be called") + } +} + func TestCLIRun_default(t *testing.T) { commandBar := new(MockCommand) commandBar.RunResult = 42 From 4bb9ba29a559edb1ee92e200cf9669a7f275e2de Mon Sep 17 00:00:00 2001 From: Katy Moe Date: Thu, 21 Apr 2022 11:58:12 +0100 Subject: [PATCH 2/2] exclude flags from subcommand parsing When determining the string to use when querying the radix tree for the longest subcommand, exclude any argument beginning with a "-", since this is a flag and not a subcommand. This fixes a bug in which "fooasdf -o=foo" is matched to the subcommand "foo" due to logic further down on line 704. --- cli.go | 8 ++++---- cli_test.go | 3 --- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/cli.go b/cli.go index 31fafa0..a2abfe1 100644 --- a/cli.go +++ b/cli.go @@ -680,12 +680,12 @@ func (c *CLI) processArgs() { } // Determine the argument we look to to end subcommands. - // We look at all arguments until one has a space. This - // disallows commands like: ./cli foo "bar baz". An argument - // with a space is always an argument. + // We look at all arguments until one is a flag or has a space. + // This disallows commands like: ./cli foo "bar baz". An + // argument with a space is always an argument. j := 0 for k, v := range c.Args[i:] { - if strings.ContainsRune(v, ' ') { + if strings.ContainsRune(v, ' ') || v[0] == '-' { break } diff --git a/cli_test.go b/cli_test.go index e381d54..6b6014e 100644 --- a/cli_test.go +++ b/cli_test.go @@ -171,9 +171,6 @@ func TestCLIRun_prefix(t *testing.T) { } } -// NOTE: This failing test is intended to demonstrate incorrect behaviour -// in which a subcommand with an arbitrary suffix is still parsed as that -// subcommand, if a supplied argument is also suffixed by that subcommand. func TestCLIRun_subcommandSuffix(t *testing.T) { buf := new(bytes.Buffer) command := new(MockCommand)