urfave / cli

A simple, fast, and fun package for building command line apps in Go
https://cli.urfave.org
MIT License
21.91k stars 1.69k forks source link

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

Closed bobheadxi closed 4 months ago

bobheadxi commented 7 months ago

My urfave/cli version is

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

Checklist

Dependency Management

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/v2@v2.25.7/context.go:167
github.com/urfave/cli/v2.checkCompletions(0x14000cfb780)
        /.asdf/installs/golang/1.20.10/packages/pkg/mod/github.com/urfave/cli/v2@v2.25.7/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/v2@v2.25.7/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/v2@v2.25.7/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/v2@v2.25.7/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/v2@v2.25.7/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/v2@v2.25.7/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
dearchap commented 7 months ago

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

bobheadxi commented 7 months ago

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

dearchap commented 7 months ago

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
bobheadxi commented 7 months ago

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

dearchap commented 4 months ago

Cant reproduce