Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to pass strings containing quotes to commands #6154

Closed
ViridiFox opened this issue Jul 27, 2022 · 0 comments · Fixed by #6161
Closed

Unable to pass strings containing quotes to commands #6154

ViridiFox opened this issue Jul 27, 2022 · 0 comments · Fixed by #6161
Labels
🐛 bug Something isn't working
Milestone

Comments

@ViridiFox
Copy link

Describe the bug

Quotes contained in a string disappear, which causes the passed string to be parsed incorrectly by a shell to which the string gets passed internally e.g. ['bash', '-c', 'echo', 'a'] instead of ['bash', '-c', 'echo a'].

How to reproduce

~
nu ❯ "bash -c 'echo a'"
bash -c 'echo a'

~
nu ❯ ^echo "bash -c 'echo a'" # problematic case
bash -c echo a

in bash:

~
bsh ❯ echo "bash -c 'echo a'"
bash -c 'echo a'

Expected behavior

for the commands' output to be the same

Screenshots

No response

Configuration

key value
version 0.66.1
branch
commit_hash
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.64.0-nightly (4d6d601c8 2022-07-26)
rust_channel nightly-x86_64-unknown-linux-gnu
cargo_version cargo 1.64.0-nightly (d8d30a753 2022-07-19)
pkg_version 0.66.1
build_time 2022-07-27 16:34:32 +00:00
build_rust_channel release
features default, trash, which, zip
installed_plugins

Additional context

No response

@hustcer hustcer added the 🐛 bug Something isn't working label Jul 28, 2022
@hustcer hustcer added this to the v0.67.0 milestone Jul 28, 2022
IanManske pushed a commit that referenced this issue May 23, 2024
This PR is a complete rewrite of `run_external.rs`. The main goal of the
rewrite is improving readability, but it also fixes some bugs related to
argument handling and the PATH variable (fixes
#6011).

I'll discuss some technical details to make reviewing easier.

## Argument handling

Quoting arguments for external commands is hard. Like, *really* hard.
We've had more than a dozen issues and PRs dedicated to quoting
arguments (see Appendix) but the current implementation is still buggy.

Here's a demonstration of the buggy behavior:

```nu
let foo = "'bar'"
^touch $foo            # This creates a file named `bar`, but it should be `'bar'`
^touch ...[ "'bar'" ]  # Same
```

I'll describe how this PR deals with argument handling.

First, we'll introduce the concept of **bare strings**. Bare strings are
**string literals** that are either **unquoted** or **quoted by
backticks** [^1]. Strings within a list literal are NOT considered bare
strings, even if they are unquoted or quoted by backticks.

When a bare string is used as an argument to external process, we need
to perform tilde-expansion, glob-expansion, and inner-quotes-removal, in
that order. "Inner-quotes-removal" means transforming from
`--option="value"` into `--option=value`.

## `.bat` files and CMD built-ins

On Windows, `.bat` files and `.cmd` files are considered executable, but
they need `CMD.exe` as the interpreter. The Rust standard library
supports running `.bat` files directly and will spawn `CMD.exe` under
the hood (see
[documentation](https://doc.rust-lang.org/std/process/index.html#windows-argument-splitting)).
However, other extensions are not supported [^2].

Nushell also supports a selected number of CMD built-ins. The problem
with CMD is that it uses a different set of quoting rules. Correctly
quoting for CMD requires using
[Command::raw_arg()](https://doc.rust-lang.org/std/os/windows/process/trait.CommandExt.html#tymethod.raw_arg)
and manually quoting CMD special characters, on top of quoting from the
Nushell side. ~~I decided that this is too complex and chose to reject
special characters in CMD built-ins instead [^3]. Hopefully this will
not affact real-world use cases.~~ I've implemented escaping that works
reasonably well.

## `which-support` feature

The `which` crate is now a hard dependency of `nu-command`, making the
`which-support` feature essentially useless. The `which` crate is
already a hard dependency of `nu-cli`, and we should consider removing
the `which-support` feature entirely.

## Appendix

Here's a list of quoting-related issues and PRs in rough chronological
order.

* #4609
* #4631
* #4601
  * #5846
* #5978
  * #6014
* #6154
  * #6161
* #6399
  * #6420
  * #6426
* #6465
* #6559
  * #6560

[^1]: The idea that backtick-quoted strings act like bare strings was
introduced by Kubouch and briefly mentioned in [the language
reference](https://www.nushell.sh/lang-guide/chapters/strings_and_text.html#backtick-quotes).

[^2]: The documentation also said "running .bat scripts in this way may
be removed in the future and so should not be relied upon", which is
another reason to move away from this. But again, quoting for CMD is
hard.

[^3]: If anyone wants to try, the best resource I found on the topic is
[this](https://daviddeley.com/autohotkey/parameters/parameters.htm).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants