Merge pull request #893 from rliebz/fs-panic

Avoid panic for missing flag value
This commit is contained in:
Ajitem Sahasrabuddhe 2019-09-14 15:35:52 +05:30 committed by GitHub
commit c0c49a10ad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 91 additions and 34 deletions

8
app.go
View File

@ -199,12 +199,12 @@ func (a *App) Run(arguments []string) (err error) {
// always appends the completion flag at the end of the command // always appends the completion flag at the end of the command
shellComplete, arguments := checkShellCompleteFlag(a, arguments) shellComplete, arguments := checkShellCompleteFlag(a, arguments)
_, err = a.newFlagSet() set, err := a.newFlagSet()
if err != nil { if err != nil {
return err return err
} }
set, err := parseIter(a, arguments[1:]) err = parseIter(set, a, arguments[1:])
nerr := normalizeFlags(a.Flags, set) nerr := normalizeFlags(a.Flags, set)
context := NewContext(a, set, nil) context := NewContext(a, set, nil)
if nerr != nil { if nerr != nil {
@ -322,12 +322,12 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) {
} }
a.Commands = newCmds a.Commands = newCmds
_, err = a.newFlagSet() set, err := a.newFlagSet()
if err != nil { if err != nil {
return err return err
} }
set, err := parseIter(a, ctx.Args().Tail()) err = parseIter(set, a, ctx.Args().Tail())
nerr := normalizeFlags(a.Flags, set) nerr := normalizeFlags(a.Flags, set)
context := NewContext(a, set, ctx) context := NewContext(a, set, ctx)

View File

@ -659,6 +659,17 @@ func TestApp_UseShortOptionHandling(t *testing.T) {
expect(t, name, expected) expect(t, name, expected)
} }
func TestApp_UseShortOptionHandling_missing_value(t *testing.T) {
app := NewApp()
app.UseShortOptionHandling = true
app.Flags = []Flag{
StringFlag{Name: "name, n"},
}
err := app.Run([]string{"", "-n"})
expect(t, err, errors.New("flag needs an argument: -n"))
}
func TestApp_UseShortOptionHandlingCommand(t *testing.T) { func TestApp_UseShortOptionHandlingCommand(t *testing.T) {
var one, two bool var one, two bool
var name string var name string
@ -688,6 +699,21 @@ func TestApp_UseShortOptionHandlingCommand(t *testing.T) {
expect(t, name, expected) expect(t, name, expected)
} }
func TestApp_UseShortOptionHandlingCommand_missing_value(t *testing.T) {
app := NewApp()
app.UseShortOptionHandling = true
command := Command{
Name: "cmd",
Flags: []Flag{
StringFlag{Name: "name, n"},
},
}
app.Commands = []Command{command}
err := app.Run([]string{"", "cmd", "-n"})
expect(t, err, errors.New("flag needs an argument: -n"))
}
func TestApp_UseShortOptionHandlingSubCommand(t *testing.T) { func TestApp_UseShortOptionHandlingSubCommand(t *testing.T) {
var one, two bool var one, two bool
var name string var name string
@ -722,6 +748,25 @@ func TestApp_UseShortOptionHandlingSubCommand(t *testing.T) {
expect(t, name, expected) expect(t, name, expected)
} }
func TestApp_UseShortOptionHandlingSubCommand_missing_value(t *testing.T) {
app := NewApp()
app.UseShortOptionHandling = true
command := Command{
Name: "cmd",
}
subCommand := Command{
Name: "sub",
Flags: []Flag{
StringFlag{Name: "name, n"},
},
}
command.Subcommands = []Command{subCommand}
app.Commands = []Command{command}
err := app.Run([]string{"", "cmd", "sub", "-n"})
expect(t, err, errors.New("flag needs an argument: -n"))
}
func TestApp_Float64Flag(t *testing.T) { func TestApp_Float64Flag(t *testing.T) {
var meters float64 var meters float64

View File

@ -193,7 +193,12 @@ func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) {
args = reorderArgs(args) args = reorderArgs(args)
} }
set, err := parseIter(c, args) set, err := c.newFlagSet()
if err != nil {
return nil, err
}
err = parseIter(set, c, args)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -70,10 +70,15 @@ func TestParseAndRunShortOpts(t *testing.T) {
{[]string{"foo", "test", "-af"}, nil, []string{}}, {[]string{"foo", "test", "-af"}, nil, []string{}},
{[]string{"foo", "test", "-cf"}, nil, []string{}}, {[]string{"foo", "test", "-cf"}, nil, []string{}},
{[]string{"foo", "test", "-acf"}, nil, []string{}}, {[]string{"foo", "test", "-acf"}, nil, []string{}},
{[]string{"foo", "test", "-invalid"}, errors.New("flag provided but not defined: -invalid"), []string{}}, {[]string{"foo", "test", "-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"}},
{[]string{"foo", "test", "-i"}, errors.New("flag needs an argument: -i"), nil},
} }
for _, c := range cases {
var args []string var args []string
cmd := Command{ cmd := Command{
Name: "test", Name: "test",
@ -89,10 +94,10 @@ func TestParseAndRunShortOpts(t *testing.T) {
BoolFlag{Name: "abc, a"}, BoolFlag{Name: "abc, a"},
BoolFlag{Name: "cde, c"}, BoolFlag{Name: "cde, c"},
BoolFlag{Name: "fgh, f"}, BoolFlag{Name: "fgh, f"},
StringFlag{Name: "ijk, i"},
}, },
} }
for _, c := range cases {
app := NewApp() app := NewApp()
app.Commands = []Command{cmd} app.Commands = []Command{cmd}

View File

@ -14,22 +14,17 @@ type iterativeParser interface {
// 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 // transforming any arguments. Otherwise, there is no way we can discriminate
// combined short options from common arguments that should be left untouched. // combined short options from common arguments that should be left untouched.
func parseIter(ip iterativeParser, args []string) (*flag.FlagSet, error) { func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error {
for { for {
set, err := ip.newFlagSet() err := set.Parse(args)
if err != nil {
return nil, err
}
err = set.Parse(args)
if !ip.useShortOptionHandling() || err == nil { if !ip.useShortOptionHandling() || err == nil {
return set, err return err
} }
errStr := err.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 { if errStr == trimmed {
return nil, err return err
} }
// regenerate the initial args with the split short opts // regenerate the initial args with the split short opts
@ -42,7 +37,7 @@ func parseIter(ip iterativeParser, args []string) (*flag.FlagSet, error) {
shortOpts := splitShortOptions(set, trimmed) shortOpts := splitShortOptions(set, trimmed)
if len(shortOpts) == 1 { if len(shortOpts) == 1 {
return nil, err return err
} }
// add each short option and all remaining arguments // add each short option and all remaining arguments
@ -50,6 +45,13 @@ func parseIter(ip iterativeParser, args []string) (*flag.FlagSet, error) {
newArgs = append(newArgs, args[i+1:]...) newArgs = append(newArgs, args[i+1:]...)
args = newArgs args = newArgs
} }
// Since custom parsing failed, replace the flag set before retrying
newSet, err := ip.newFlagSet()
if err != nil {
return err
}
*set = *newSet
} }
} }