From 8a3a4f3b623a758e0d9910f9fd93c0794659fbce Mon Sep 17 00:00:00 2001 From: Gavin Panella Date: Fri, 7 Aug 2026 23:29:39 +0200 Subject: [PATCH 1/2] Quote fewer characters, i.e. `:`, `@`, and `+` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously any one of these dragged a whole word into quotes, so a URL came out as `https'://github.com/allenap/shell-quote'`. That is correct but noisy, which rather undermines the claim that `Sh` output is "better for humans to read, to copy and paste". All three are inert wherever they land in a word, checked in leading, medial, and trailing position against `/bin/sh`, Bash 3.2 and 5.3, Dash, Z Shell 5.9, and fish 4.8. `%` and `=` were tried too, and would have addressed #8 as well, but they earned a guard that grew a new special case each time I looked at it – append assignments, `=` expansion, `MAGIC_EQUAL_SUBST`, job specifications – and two of those were found only after I had convinced myself the thing was correct. For a crate whose one job is to emit a single literal word, terser output is not worth that. They stay quoted, and the module documentation in `ascii.rs` now records every hazard found, and the reason a plain non-interactive `sh -c` probe fails to reveal them, so that the next person to try this starts where I left off rather than where I began. Closes #42. --- src/ascii.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++-- src/utf8.rs | 13 +++++++++-- tests/test_bash.rs | 19 +++++++++++++++ tests/test_fish.rs | 18 ++++++++++++++ tests/test_sh.rs | 18 ++++++++++++++ 5 files changed, 122 insertions(+), 4 deletions(-) diff --git a/src/ascii.rs b/src/ascii.rs index d81cf7e..3792153 100644 --- a/src/ascii.rs +++ b/src/ascii.rs @@ -3,6 +3,50 @@ //! Scanner for ASCII control codes, shell metacharacters, printable characters, //! and extended codes, i.e. classify each byte in a stream according to where //! it appears in extended ASCII. +//! +//! # On widening the inert set +//! +//! [`Char::PrintableInert`] means "safe to emit bare, wherever this byte lands +//! in a word, in every shell we support". Moving a byte into it makes output +//! terser, which is nice, but the cost of being wrong is that we emit something +//! the shell does not read as a single literal word – so the bar is high. +//! +//! `:`, `@`, and `+` clear it. Each was checked in leading, medial, and +//! trailing position against `/bin/sh`, Bash 3.2 and 5.3, Dash, Z Shell 5.9, +//! and fish 4.8. +//! +//! `%` and `=` were tried and rejected. Both are inert in most positions, and +//! both could in principle be handled by tracking position within the word, but +//! between them they attracted this list, and there is no reason to think it is +//! complete: +//! +//! - `FOO=bar` is an assignment rather than a word in Bourne-like shells, and +//! in fish it is an outright error, "Unsupported use of '='". +//! - `FOO+=bar` is an _append_ assignment in Bash and Z Shell. Note that `+` is +//! itself inert, so any position-tracking scheme has to know that a name may +//! be followed by an optional `+` before the `=`. This one was missed first +//! time round. +//! - `=foo` is subject to `=` expansion in Z Shell, where `EQUALS` is on by +//! default. This matters here because `Zsh` is an alias for `Bash`. +//! - With `MAGIC_EQUAL_SUBST` set, Z Shell expands after _any_ `=` in _any_ +//! word: `--arg=~root` becomes `--arg=/var/root`. We survive that only +//! because `~`, `$`, `*`, `?`, and `[` are all still quoted; keep it that way. +//! - `%1` in command position is a job specification. In Bash **no quoting +//! helps** – see the warning on [`crate::Bash`] – but in Z Shell quoting does +//! help, so making `%` inert would take something Z Shell can be protected +//! from and make it unfixable there too. +//! +//! For balance, these look dangerous and are not, all checked against Bash and +//! Z Shell: `--arg=var`, `a.b=c`, `1=x`, `a+b=c`, and `FOO++=bar` cannot be read +//! as assignments; and `-=`, `*=`, `/=`, `%=`, `<<=`, `>>=`, `&=`, `^=`, `|=` +//! are _arithmetic_ operators, meaningful only inside `(( … ))`, `let`, and +//! `$(( … ))`, never at the level of a word. +//! +//! The lesson worth keeping: shell behaviour here is conditional on shell +//! options (`EXTENDED_GLOB`, `MAGIC_EQUAL_SUBST`), on job control, and on +//! whether the shell is interactive – none of which a plain non-interactive +//! `sh -c` test exercises. Probing that way will under-report. It is how both +//! `FOO+=bar` and `%1` were missed on the first attempt. use std::borrow::Borrow; @@ -56,11 +100,21 @@ impl Char { b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' => PrintableInert(ch), b',' | b'.' | b'/' | b'_' | b'-' => PrintableInert(ch), + // ASCII punctuation which is inert wherever it lands in a word, in + // every shell this crate supports – checked in leading, medial, and + // trailing position against `/bin/sh`, Bash 3.2 and 5.3, Dash, Z + // Shell 5.9, and fish 4.8. + b':' | b'@' | b'+' => PrintableInert(ch), + // ASCII punctuation which can have significance in the shell. b'|' | b'&' | b';' | b'(' | b')' | b'<' | b'>' => Printable(ch), b' ' | b'?' | b'[' | b']' | b'{' | b'}' | b'`' => Printable(ch), - b'~' | b'!' | b'$' | b'@' | b'+' | b'=' | b'*' => Printable(ch), - b'%' | b'#' | b':' | b'^' => Printable(ch), + b'~' | b'!' | b'$' | b'*' | b'#' | b'^' => Printable(ch), + + // These two look inert and are not; they are quoted deliberately, + // and the module documentation above says at length why, so that + // nobody has to rediscover it. + b'%' | b'=' => Printable(ch), // ASCII extended characters, or high bytes. 0x80..=0xff => Extended(ch), diff --git a/src/utf8.rs b/src/utf8.rs index 6392502..81d5add 100644 --- a/src/utf8.rs +++ b/src/utf8.rs @@ -55,11 +55,20 @@ impl Char { b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' => PrintableInert(ascii), b',' | b'.' | b'/' | b'_' | b'-' => PrintableInert(ascii), + // ASCII punctuation which is inert wherever it lands in a word, + // in every shell this crate supports. See the equivalent arm in + // [`crate::ascii`] for how this was established, and for why + // `%` and `=` are not among them. + b':' | b'@' | b'+' => PrintableInert(ascii), + // ASCII punctuation which can have significance in the shell. b'|' | b'&' | b';' | b'(' | b')' | b'<' | b'>' => Printable(ascii), b' ' | b'?' | b'[' | b']' | b'{' | b'}' | b'`' => Printable(ascii), - b'~' | b'!' | b'$' | b'@' | b'+' | b'=' | b'*' => Printable(ascii), - b'%' | b'#' | b':' | b'^' => Printable(ascii), + b'~' | b'!' | b'$' | b'*' | b'#' | b'^' => Printable(ascii), + + // These two look inert and are not; see the module + // documentation in [`crate::ascii`] for why. + b'%' | b'=' => Printable(ascii), // UTF-8 sequences. 0x80..=0xff => Utf8(ch), diff --git a/tests/test_bash.rs b/tests/test_bash.rs index f1f00f5..3d98153 100644 --- a/tests/test_bash.rs +++ b/tests/test_bash.rs @@ -42,6 +42,25 @@ mod bash_impl { assert_eq!(Bash::quote_vec("Hello \r\n"), b"$'Hello \\r\\n'"); } + /// `:`, `@`, and `+` are inert wherever they land in a word, so URLs and + /// the like are passed through untouched, i.e. ANSI-C quoting is not + /// applied when there is nothing to escape; see + /// . + #[test] + fn test_inert_punctuation_is_not_quoted() { + assert_eq!( + Bash::quote_vec("https://github.com/RazrFalcon/pico-args"), + b"https://github.com/RazrFalcon/pico-args" + ); + assert_eq!(Bash::quote_vec("user@example.com"), b"user@example.com"); + assert_eq!(Bash::quote_vec("1.2.3+build"), b"1.2.3+build"); + assert_eq!(Bash::quote_vec(":@+"), b":@+"); + // `%` and `=` are deliberately _not_ inert; see the note against them + // in `src/ascii.rs`. + assert_eq!(Bash::quote_vec("FOO=bar"), b"$'FOO=bar'"); + assert_eq!(Bash::quote_vec("%1"), b"$'%1'"); + } + #[test] fn test_empty_string() { assert_eq!(Bash::quote_vec(""), b"''"); diff --git a/tests/test_fish.rs b/tests/test_fish.rs index 4c0e027..52c59f1 100644 --- a/tests/test_fish.rs +++ b/tests/test_fish.rs @@ -75,6 +75,24 @@ mod fish_impl { assert_eq!(Fish::quote_vec("Hello \r\n"), b"Hello' '\\r\\n"); } + /// `:`, `@`, and `+` are inert wherever they land in a word, so URLs and + /// the like are passed through untouched; see + /// . + #[test] + fn test_inert_punctuation_is_not_quoted() { + assert_eq!( + Fish::quote_vec("https://github.com/RazrFalcon/pico-args"), + b"https://github.com/RazrFalcon/pico-args" + ); + assert_eq!(Fish::quote_vec("user@example.com"), b"user@example.com"); + assert_eq!(Fish::quote_vec("1.2.3+build"), b"1.2.3+build"); + assert_eq!(Fish::quote_vec(":@+"), b":@+"); + // `%` and `=` are deliberately _not_ inert; see the note against them + // in `src/ascii.rs`. + assert_eq!(Fish::quote_vec("FOO=bar"), b"FOO'=bar'"); + assert_eq!(Fish::quote_vec("%1"), b"'%1'"); + } + #[test] fn test_empty_string() { assert_eq!(Fish::quote_vec(""), b"''"); diff --git a/tests/test_sh.rs b/tests/test_sh.rs index 424f190..02b76ba 100644 --- a/tests/test_sh.rs +++ b/tests/test_sh.rs @@ -70,6 +70,24 @@ mod sh_impl { assert_eq!(Sh::quote_vec("Hello \r\n"), b"Hello' \r\n'"); } + /// `:`, `@`, and `+` are inert wherever they land in a word, so URLs and + /// the like are passed through untouched; see + /// . + #[test] + fn test_inert_punctuation_is_not_quoted() { + assert_eq!( + Sh::quote_vec("https://github.com/RazrFalcon/pico-args"), + b"https://github.com/RazrFalcon/pico-args" + ); + assert_eq!(Sh::quote_vec("user@example.com"), b"user@example.com"); + assert_eq!(Sh::quote_vec("1.2.3+build"), b"1.2.3+build"); + assert_eq!(Sh::quote_vec(":@+"), b":@+"); + // `%` and `=` are deliberately _not_ inert; see the note against them + // in `src/ascii.rs`. + assert_eq!(Sh::quote_vec("FOO=bar"), b"FOO'=bar'"); + assert_eq!(Sh::quote_vec("%1"), b"'%1'"); + } + #[test] fn test_empty_string() { assert_eq!(Sh::quote_vec(""), b"''"); From 5bfa942eea6261b71254974769fee2c117d601f1 Mon Sep 17 00:00:00 2001 From: Gavin Panella Date: Fri, 7 Aug 2026 23:30:20 +0200 Subject: [PATCH 2/2] Warn that a leading `%` is unsafe as a command name in Bash In command position Bash reads a word beginning with `%` as a job specification, so `%1` on its own means `fg %1`, and quoting does not prevent it: Bash tests the word's value, after expansion and quote removal, so `%1`, `'%1'`, `"%1"`, `$'%1'`, `\%1`, and `""%1` all alike run `fg`. Nor does it depend on job control being enabled, which was my first guess. Bash rewrites the word either way and only the complaint differs, `fg: no job control` or `fg: %1: no such job`; the same holds with `set -m`, with `set +m`, under `--posix`, and in Bash 3.2. An executable named `%1` sitting on `PATH` will not be run, though it runs happily by an explicit path. Only the first character counts: `a%1` is an ordinary command name. Z Shell tests the literal token instead, so quoting does work there. That is why `%` is quoted unconditionally rather than only when it leads a word: it is what Z Shell needs, and it is at worst harmless in Bash. Worth keeping in proportion: this bites only in command position. As an argument, which is what this crate is mostly used for, `%` is an ordinary character in both shells. --- src/bash.rs | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/bash.rs b/src/bash.rs index 63ba917..8b98906 100644 --- a/src/bash.rs +++ b/src/bash.rs @@ -8,7 +8,7 @@ use crate::{Quotable, QuoteInto}; /// /// Quoted/escaped strings produced by [`Bash`] work in both Bash and Z Shell. /// -/// # ⚠️ Warning +/// # ⚠️ Warning regarding `NUL` /// /// It is _possible_ to encode NUL in a Bash string, but Bash appears to then /// truncate the rest of the string after that point **or** sometimes it filters @@ -22,6 +22,48 @@ use crate::{Quotable, QuoteInto}; /// /// [modified-utf-8]: https://en.wikipedia.org/wiki/UTF-8#Modified_UTF-8 /// +/// # ⚠️ Warning regarding `%` and job control in Bash +/// +/// A word beginning with `%` **cannot be made safe** to use as a **command +/// name** in Bash, by this crate or by any other means. In command position +/// Bash reads such a word as a [job specification][job-control]: `%1` on its +/// own is shorthand for `fg %1`. +/// +/// Quoting does not prevent it, because Bash tests the word's _value_, after +/// expansion and quote removal. Every one of these runs `fg`: +/// +/// ```bash +/// %1 '%1' "%1" $'%1' \%1 ""%1 +/// ``` +/// +/// Nor is it conditional on job control being enabled. Bash rewrites the word +/// either way; all that changes is which complaint you get, `fg: no job +/// control` or `fg: %1: no such job`. It behaves this same way with job control +/// off, with `set -m`, with `set +m`, under `--posix`, and in Bash 3.2 as +/// shipped by Apple. Put an executable named `%1` on `PATH` and Bash will not +/// run it, though it will run it happily by an explicit path. +/// +/// It is the first character that matters, and only the first: `%1`, `%foo`, +/// `%`, `%%`, `%+`, `%-`, and `%?x` are all intercepted, while `a%1` is an +/// ordinary command name. +/// +/// Z Shell differs: it tests the literal token, so there quoting _does_ prevent +/// it, and all of the above but the first are ordinary command names. This +/// crate quotes `%` unconditionally, which is what Z Shell needs, and is at +/// worst harmless in Bash. +/// +/// Keep it in proportion, though: this bites only in command position. As an +/// **argument** – which is what this crate is mostly used for – `%` is an +/// ordinary character in both shells, and `printf %s $'%1'` prints `%1` as you +/// would expect. +/// +/// If you are interpolating an untrusted string into command position, quoting +/// is not sufficient protection in any case; prefer to invoke a known command +/// and pass the string as an argument. +/// +/// [job-control]: +/// https://www.gnu.org/software/bash/manual/html_node/Job-Control-Basics.html +/// /// # Notes /// /// From bash(1):