x/urfave-cli
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Avoid panic for missing flag value

Browse Source 
Currently, in cases where a flag value is required but not passed and
short-option handling is enabled, a panic will occur due to a nil
pointer dereference. This prevents that situation from occurring,
instead propagating the appropriate error.
This commit is contained in:
Robert Liebowitz 2019-09-13 05:30:07 -04:00 committed by Ajitem Sahasrabuddhe
parent f12b8ca4e5
commit 7d46b6d7f1
No known key found for this signature in database
GPG Key ID: 5B0EE10DAA76876C
5 changed files with 99 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
app.go
View File

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

61
app_test.go
View File

@ -239,11 +239,11 @@ func ExampleApp_Run_bashComplete_withShortFlag() {
app.EnableBashCompletion = true
app.Flags = []Flag{
&IntFlag{
Name: "other",
Name: "other",
Aliases: []string{"o"},
},
&StringFlag{
Name: "xyz",
Name: "xyz",
Aliases: []string{"x"},
},
}
@ -268,11 +268,11 @@ func ExampleApp_Run_bashComplete_withLongFlag() {
app.EnableBashCompletion = true
app.Flags = []Flag{
&IntFlag{
Name: "other",
Name: "other",
Aliases: []string{"o"},
},
&StringFlag{
Name: "xyz",
Name: "xyz",
Aliases: []string{"x"},
},
&StringFlag{
@ -296,11 +296,11 @@ func ExampleApp_Run_bashComplete_withMultipleLongFlag() {
app.EnableBashCompletion = true
app.Flags = []Flag{
&IntFlag{
Name: "int-flag",
Name: "int-flag",
Aliases: []string{"i"},
},
&StringFlag{
Name: "string",
Name: "string",
Aliases: []string{"s"},
},
&StringFlag{
@ -326,7 +326,7 @@ func ExampleApp_Run_bashComplete() {
os.Args = []string{"greet", "--generate-bash-completion"}
app := &App{
Name: "greet",
Name: "greet",
EnableBashCompletion: true,
Commands: []*Command{
{
@ -638,6 +638,17 @@ func TestApp_UseShortOptionHandling(t *testing.T) {
expect(t, name, expected)
}
func TestApp_UseShortOptionHandling_missing_value(t *testing.T) {
app := NewApp()
app.UseShortOptionHandling = true
app.Flags = []Flag{
&StringFlag{Name: "name", Aliases: []string{"n"}},
}
err := app.Run([]string{"", "-n"})
expect(t, err, errors.New("flag needs an argument: -n"))
}
func TestApp_UseShortOptionHandlingCommand(t *testing.T) {
var one, two bool
var name string
@ -667,6 +678,21 @@ func TestApp_UseShortOptionHandlingCommand(t *testing.T) {
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", Aliases: []string{"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) {
var one, two bool
var name string
@ -692,7 +718,7 @@ func TestApp_UseShortOptionHandlingSubCommand(t *testing.T) {
},
}
command.Subcommands = []*Command{subCommand}
app.Commands = []*Command{command}
app.Commands = []*Command{command}
err := app.Run([]string{"", "cmd", "sub", "-on", expected})
expect(t, err, nil)
@ -701,6 +727,25 @@ func TestApp_UseShortOptionHandlingSubCommand(t *testing.T) {
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", Aliases: []string{"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) {
var meters float64

12
command.go
View File

@ -175,16 +175,16 @@ func (c *Command) useShortOptionHandling() bool {
}
func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) {
if c.SkipFlagParsing {
set, err := c.newFlagSet()
if err != nil {
return nil, err
}
set, err := c.newFlagSet()
if err != nil {
return nil, err
}
if c.SkipFlagParsing {
return set, set.Parse(append([]string{"--"}, args.Tail()...))
}
set, err := parseIter(c, args.Tail())
err = parseIter(set, c, args.Tail())
if err != nil {
return nil, err
}

43
command_test.go
View File

@ -52,7 +52,7 @@ func TestParseAndRunShortOpts(t *testing.T) {
cases := []struct {
testArgs args
expectedErr error
expectedArgs *args
expectedArgs Args
}{
{testArgs: args{"foo", "test", "-a"}, expectedErr: nil, expectedArgs: &args{}},
{testArgs: args{"foo", "test", "-c", "arg1", "arg2"}, expectedErr: nil, expectedArgs: &args{"arg1", "arg2"}},
@ -61,28 +61,33 @@ func TestParseAndRunShortOpts(t *testing.T) {
{testArgs: args{"foo", "test", "-af"}, expectedErr: nil, expectedArgs: &args{}},
{testArgs: args{"foo", "test", "-cf"}, expectedErr: nil, expectedArgs: &args{}},
{testArgs: args{"foo", "test", "-acf"}, expectedErr: nil, expectedArgs: &args{}},
{testArgs: args{"foo", "test", "-invalid"}, expectedErr: errors.New("flag provided but not defined: -invalid"), expectedArgs: &args{}},
{testArgs: args{"foo", "test", "-invalid"}, expectedErr: errors.New("flag provided but not defined: -invalid"), expectedArgs: nil},
{testArgs: args{"foo", "test", "-acf", "arg1", "-invalid"}, expectedErr: nil, expectedArgs: &args{"arg1", "-invalid"}},
}
var args Args
cmd := &Command{
Name: "test",
Usage: "this is for testing",
Description: "testing",
Action: func(c *Context) error {
args = c.Args()
return nil
},
UseShortOptionHandling: true,
Flags: []Flag{
&BoolFlag{Name: "abc", Aliases: []string{"a"}},
&BoolFlag{Name: "cde", Aliases: []string{"c"}},
&BoolFlag{Name: "fgh", Aliases: []string{"f"}},
},
{testArgs: args{"foo", "test", "-acfi", "not-arg", "arg1", "-invalid"}, expectedErr: nil, expectedArgs: &args{"arg1", "-invalid"}},
{testArgs: args{"foo", "test", "-i", "ivalue"}, expectedErr: nil, expectedArgs: &args{}},
{testArgs: args{"foo", "test", "-i", "ivalue", "arg1"}, expectedErr: nil, expectedArgs: &args{"arg1"}},
{testArgs: args{"foo", "test", "-i"}, expectedErr: errors.New("flag needs an argument: -i"), expectedArgs: nil},
}
for _, c := range cases {
var args Args
cmd := &Command{
Name: "test",
Usage: "this is for testing",
Description: "testing",
Action: func(c *Context) error {
args = c.Args()
return nil
},
UseShortOptionHandling: true,
Flags: []Flag{
&BoolFlag{Name: "abc", Aliases: []string{"a"}},
&BoolFlag{Name: "cde", Aliases: []string{"c"}},
&BoolFlag{Name: "fgh", Aliases: []string{"f"}},
&StringFlag{Name: "ijk", Aliases:[]string{"i"}},
},
}
app := NewApp()
app.Commands = []*Command{cmd}

22
parse.go
View File

@ -14,22 +14,17 @@ 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.
func parseIter(ip iterativeParser, args []string) (*flag.FlagSet, error) {
func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error {
for {
set, err := ip.newFlagSet()
if err != nil {
return nil, err
}
err = set.Parse(args)
err := set.Parse(args)
if !ip.useShortOptionHandling() || err == nil {
return set, err
return err
}
errStr := err.Error()
trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: ")
if errStr == trimmed {
return nil, err
return err
}
// 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)
if len(shortOpts) == 1 {
return nil, err
return err
}
// 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:]...)
args = newArgs
}
// Since custom parsing failed, replace the flag set before retrying
newSet, err := ip.newFlagSet()
if err != nil {
return err
}
*set = *newSet
}
}