From 8486894af8b949f9586bd5c2ddef135f3280b1cb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 8 May 2020 12:20:27 +0200 Subject: [PATCH] Fix regression in handling flag-values starting with hyphen 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 --- command.go | 2 +- command_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index 24e9e5c572..74fe7f4c4c 100644 --- a/command.go +++ b/command.go @@ -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) diff --git a/command_test.go b/command_test.go index a59d368c46..b8d6c99fdd 100644 --- a/command_test.go +++ b/command_test.go @@ -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{