clap-rs / clap

A full featured, fast Command Line Argument Parser for Rust
docs.rs/clap
Apache License 2.0
14.31k stars 1.05k forks source link

Special characters not escaped for zsh completion script #1596

Open iyzana opened 5 years ago

iyzana commented 5 years ago

Rust Version

rustc 1.38.0 (625451e37 2019-09-23)

Affected Version of clap

clap 2.33.0

Expected Behavior Summary

Generated Zsh completions correctly complete possible_values even in the presence of characters that have a special meaning for the shell

Actual Behavior Summary

For example, when some string in possible_values contains | (a pipe) the generated completion script fails with

(eval):1: parse error near `|'

when pressing tab for that argument. That was the problem I ran into. I then tested some other special characters.

A sample of the characters I found to have problems:

There are more special characters, that need to be escaped. The reproduction from the repo tests against all non control ascii characters and some dollar-something strings.

Steps to Reproduce the issue

Sample Code or Link to Sample Code

https://github.com/succcubbus/clap-zsh-completions-repro

Debug output

https://pastebin.com/KU4yd6FR

dotboris commented 5 years ago

I've hit the same issue in a project, I've had to work around this by rephrasing the about of a subcommand. https://github.com/dotboris/alt/commit/37e3255a6d154a57f9cb0ca00150e25312e253b5

Nukesor commented 4 years ago

' also breaks completion when using subcommands, which uses " for strings).

alerque commented 4 years ago

We just hit this in a project too, backticks in a documentation string shouldn't be causing code execution!

In our case we're using the 3.0.0-beta.2 release, so this isn't just 2.x stuff.

https://github.com/theleagueof/fontship/pull/108

pksunkara commented 4 years ago

Unfortunately I am still unable to get zsh working properly on my computer. So, I am going to have to rely on contribution

alerque commented 4 years ago

Would introducing a dependency for this be acceptable? I had a look at what's available now and this would take quite a bit of coding. Most of the current dependencies are focused on Claps' core function: accepting stuff in from the CLI. Passing stuff out to into the CLI (as opposed to just the TTY) would is a bit of a different problem but it seems reasonable to expect Clap to handle this robustly. As the inital report and demo repository note there are a lot of cases involved here. I haven't confirmed if shell-escape is actually appropriate or if there are better alternatives, but before I research too much lets hear about whether a dependency to safely handle shell escapeing is going to be allowed.

pksunkara commented 4 years ago

Since the generator lives in a separate crate and is always opt-in, I would tentatively say yes.

pksunkara commented 4 years ago

But I don't think there are any crates that do this.