Quote fewer characters, i.e. :, @, and +

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),

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):

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),

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
+    /// <https://github.com/allenap/shell-quote/issues/42>.
+    #[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"''");

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
+    /// <https://github.com/allenap/shell-quote/issues/42>.
+    #[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"''");

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
+    /// <https://github.com/allenap/shell-quote/issues/42>.
+    #[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"''");