cli icon indicating copy to clipboard operation
cli copied to clipboard

v2: if custom subcommand completion is provided, completing after partial flag input panics

Open bobheadxi opened this issue 1 year ago • 4 comments

My urfave/cli version is

https://github.com/urfave/cli/releases/tag/v2.25.7

Checklist

  • [x] Are you running the latest v2 release? The list of releases is here.
  • [x] Did you check the manual for your release? The v2 manual is here
  • [x] Did you perform a search about this problem? Here's the GitHub guide about searching.

Dependency Management

  • My project is using go modules.

Describe the bug

When providing a custom completions func for a subcommand, and typing an invalid flag, the completion will panic. This seems to be caused here, where flagSet may be nil if err != nil, but flagSet is referenced in checkCompletions:

https://github.com/urfave/cli/blob/c023d9bc5a3122830c9355a0a8c17137e0c8556f/command.go#L155-L160

To reproduce

In a subcommand:

Flags: []cli.Flag{
  &cli.StringFlag{
    Name:  "stringflag",
  },
},
BashComplete: func(...) { ... },

Attempt tab completion on:

mycommand subcommand --stringflag 

Observed behavior

mycommand subcommand --stringflag panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x30 pc=0x10441912c]

goroutine 1 [running]:
flag.(*FlagSet).Args(...)
        /.asdf/installs/golang/1.20.10/go/src/flag/flag.go:709
github.com/urfave/cli/v2.(*Context).Args(...)
        /.asdf/installs/golang/1.20.10/packages/pkg/mod/github.com/urfave/cli/[email protected]/context.go:167
github.com/urfave/cli/v2.checkCompletions(0x14000cfb780)
        /.asdf/installs/golang/1.20.10/packages/pkg/mod/github.com/urfave/cli/[email protected]/help.go:467 +0x2c
github.com/urfave/cli/v2.(*Command).Run(0x140005b3340, 0x14000cfb780, {0x14000225300, 0x2, 0x2})
        /.asdf/installs/golang/1.20.10/packages/pkg/mod/github.com/urfave/cli/[email protected]/command.go:158 +0xe8
github.com/urfave/cli/v2.(*Command).Run(0x140005b31e0, 0x14000cfb6c0, {0x14001efcff0, 0x3, 0x3})
        /.asdf/installs/golang/1.20.10/packages/pkg/mod/github.com/urfave/cli/[email protected]/command.go:267 +0x97c
github.com/urfave/cli/v2.(*Command).Run(0x10dbe2600, 0x14000cfb500, {0x14000cfb540, 0x4, 0x4})
        /.asdf/installs/golang/1.20.10/packages/pkg/mod/github.com/urfave/cli/[email protected]/command.go:267 +0x97c
github.com/urfave/cli/v2.(*Command).Run(0x14001f01ce0, 0x14000cfb100, {0x14000130120, 0x5, 0x6})
        /.asdf/installs/golang/1.20.10/packages/pkg/mod/github.com/urfave/cli/[email protected]/command.go:267 +0x97c
github.com/urfave/cli/v2.(*App).RunContext(0x10dbeb9e0, {0x109522728?, 0x14000128018}, {0x14000130120, 0x6, 0x6})
        /.asdf/installs/golang/1.20.10/packages/pkg/mod/github.com/urfave/cli/[email protected]/app.go:332 +0x604
main.main()
        /Projects/sourcegraph/sourcegraph/dev/sg/main.go:36 +0xb8
Ter

Expected behavior

Not a panic :) If anything, would be nice to be able to provide completions on string flag

Run go version and paste its output here

go version go1.20.10 darwin/arm64

bobheadxi avatar Nov 24 '23 21:11 bobheadxi

@bobheadxi Can you provide an example of what you are doing in the completion func ?

dearchap avatar Nov 24 '23 22:11 dearchap

Can you provide an example of what you are doing in the completion func ?

The bug occurs even if you don't do anything in the completion func directly (see description - it seems to happen in urfave/cli before the handler actually executes), but https://github.com/sourcegraph/sourcegraph/pull/58569 has a concrete example

bobheadxi avatar Nov 28 '23 00:11 bobheadxi

The reason I ask is that I dont see the issue when the completion func is not set. Let me investigate further. I have specified an invalid flag on the command line and then tried completion of a valid flag and it works just fine

$ issue_1822 command subcommand --stss --
--help        --stringflag  
$ issue_1822 command subcommand --stss --stringflag

dearchap avatar Nov 28 '23 00:11 dearchap

Hm, now that I'm taking a closer look it seems I can't reproduce it either:

	if err := (&cli.App{
		Name:                 "sg",
		EnableBashCompletion: true,
		Commands: []*cli.Command{{
			Name: "bar",
			Flags: []cli.Flag{
				&cli.StringFlag{
					Name:  "stringflag",
					Value: "vcs",
				},
			},
// internal helper used in my panic scenario
			BashComplete: completions.CompletePositionalArgs(func(args cli.Args) (options []string) {
				return []string{args.First()}
			}),
			Action: func(ctx *cli.Context) error {
				println("action!")
				return nil
			},
		}},
	}).RunContext(context.Background(), os.Args); err != nil {
		println(err.Error())
		os.Exit(1)
	}

However, in our full CLI app, the panic continues to point to the same place in the issue description, i.e. https://github.com/urfave/cli/blob/c023d9bc5a3122830c9355a0a8c17137e0c8556f/help.go#L467 , and the stacktrace looks like the one I shared previously. We do some pretty extensive setup so there might be an issue deep in there (though we skip most setup if we detect the bash completion flag), but the stacktrace doesn't seem to indicate that - let me dig into this some more

bobheadxi avatar Nov 28 '23 03:11 bobheadxi

Cant reproduce

dearchap avatar Feb 18 '24 19:02 dearchap