Skip to content

Commit

Permalink
Fix regression in handling flag-values starting with hyphen
Browse files Browse the repository at this point in the history
Fix for a regression between v1.22.1 and v1.22.2, where
flag values starting with a hyphen would be parsed as a flag:

    runc update test_update --memory-swap -1
    Incorrect Usage: flag provided but not defined: -1

This problem was caused by `reorderArgs()` not properly checking
if the next arg in the list was a value for the flag (and not
the next flag);

Before this patch:

    args before reordering: --opt,--opt-value,arg1
    args after reordering:  --opt,arg1,--opt-value
    args before reordering: --opt,--opt-value,arg1,arg2
    args after reordering:  --opt,arg1,--opt-value,arg2
    args before reordering: arg1,--opt,--opt-value,arg2
    args after reordering:  --opt,arg2,arg1,--opt-value

After this patch is applied:

    args before reordering: --opt,--opt-value,arg1
    args after reordering:  --opt,--opt-value,arg1
    args before reordering: --opt,--opt-value,arg1,arg2
    args after reordering:  --opt,--opt-value,arg1,arg2
    args before reordering: arg1,--opt,--opt-value,arg2
    args after reordering:  --opt,--opt-value,arg1,arg2

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
thaJeztah committed May 8, 2020
1 parent 1e44266 commit 8486894
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
2 changes: 1 addition & 1 deletion command.go
Expand Up @@ -235,7 +235,7 @@ func reorderArgs(commandFlags []Flag, args []string) []string {
break

// checks if this arg is a value that should be re-ordered next to its associated flag
} else if nextIndexMayContainValue && !strings.HasPrefix(arg, "-") {
} else if nextIndexMayContainValue && !argIsFlag(commandFlags, arg) {
nextIndexMayContainValue = false
reorderedArgs = append(reorderedArgs, arg)

Expand Down
66 changes: 66 additions & 0 deletions command_test.go
Expand Up @@ -114,6 +114,72 @@ func TestParseAndRunShortOpts(t *testing.T) {
}
}

// test-case for https://github.com/urfave/cli/issues/1092
func TestParseAndRunHyphenValues(t *testing.T) {
cases := []struct {
testArgs []string
expectedArgs []string
expectedOpt string
}{
{[]string{"foo", "test", "arg1"}, []string{"arg1"}, ""},
{[]string{"foo", "test", "arg1", "arg2"}, []string{"arg1", "arg2"}, ""},
{[]string{"foo", "test", "--opt", "opt-value"}, []string{}, "opt-value"},
{[]string{"foo", "test", "--opt", "opt-value", "arg1"}, []string{"arg1"}, "opt-value"},
{[]string{"foo", "test", "--opt", "opt-value", "arg1", "arg2"}, []string{"arg1", "arg2"}, "opt-value"},
{[]string{"foo", "test", "arg1", "--opt", "opt-value"}, []string{"arg1"}, "opt-value"},
{[]string{"foo", "test", "arg1", "--opt", "opt-value", "arg2"}, []string{"arg1", "arg2"}, "opt-value"},

{[]string{"foo", "test", "--opt", "-opt-value"}, []string{}, "-opt-value"},
{[]string{"foo", "test", "--opt", "-opt-value", "arg1"}, []string{"arg1"}, "-opt-value"},
{[]string{"foo", "test", "--opt", "-opt-value", "arg1", "arg2"}, []string{"arg1", "arg2"}, "-opt-value"},
{[]string{"foo", "test", "arg1", "--opt", "-opt-value"}, []string{"arg1"}, "-opt-value"},
{[]string{"foo", "test", "arg1", "--opt", "-opt-value", "arg2"}, []string{"arg1", "arg2"}, "-opt-value"},

{[]string{"foo", "test", "--opt", "--opt-value"}, []string{}, "--opt-value"},
{[]string{"foo", "test", "--opt", "--opt-value", "arg1"}, []string{"arg1"}, "--opt-value"},
{[]string{"foo", "test", "--opt", "--opt-value", "arg1", "arg2"}, []string{"arg1", "arg2"}, "--opt-value"},
{[]string{"foo", "test", "arg1", "--opt", "--opt-value"}, []string{"arg1"}, "--opt-value"},
{[]string{"foo", "test", "arg1", "--opt", "--opt-value", "arg2"}, []string{"arg1", "arg2"}, "--opt-value"},
}

for _, tc := range cases {
tc := tc
t.Run(strings.Join(tc.testArgs, "_"), func(t *testing.T){
var (
args []string
opt string
)

cmd := Command{
Name: "test",
Usage: "this is for testing",
Description: "testing",
Action: func(c *Context) error {
args = c.Args()
opt = c.String("opt")
return nil
},
Flags: []Flag{StringFlag{
Name: "opt",
Usage: "opt; set '-1' to disable",
}},
}

app := NewApp()
app.Writer = ioutil.Discard
app.ErrWriter = ioutil.Discard
app.Commands = []Command{cmd}

err := app.Run(tc.testArgs)

expect(t, err, nil)
expect(t, args, tc.expectedArgs)
expect(t, opt, tc.expectedOpt)
})

}
}

func TestCommand_Run_DoesNotOverwriteErrorFromBefore(t *testing.T) {
app := NewApp()
app.Commands = []Command{
Expand Down

0 comments on commit 8486894

Please sign in to comment.