From 9a24cbc22483a79611c07cd9708984c53fd5c7c4 Mon Sep 17 00:00:00 2001 From: Gavin Panella Date: Wed, 3 Jul 2024 23:33:53 +0200 Subject: [PATCH] Remove `String` support because `Sh` breaks assumptions re. UTF-8 Hopefully this is temporary. --- .vscode/settings.json | 1 + README.md | 4 ++-- src/lib.rs | 26 +------------------------- tests/test_bash.rs | 16 ++++++++++------ tests/test_fish.rs | 16 ++++++++++------ tests/test_sh.rs | 16 ++++++++++------ tests/test_ux.rs | 22 ---------------------- 7 files changed, 34 insertions(+), 67 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index d2232c8..8551871 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -4,6 +4,7 @@ "backquote", "bindir", "bstr", + "bstring", "Clippy", "dollarsign", "dtolnay", diff --git a/README.md b/README.md index 85aa834..ecffe95 100644 --- a/README.md +++ b/README.md @@ -79,9 +79,9 @@ Or the extension trait [`QuoteExt`] for pushing quoted strings into a buffer: ```rust use shell_quote::{Bash, QuoteExt}; -let mut script: String = "echo ".into(); +let mut script: bstr::BString = "echo ".into(); script.push_quoted(Bash, "foo bar"); -script.push_str(" > "); +script.extend(b" > "); script.push_quoted(Bash, "/path/(to)/[output]"); assert_eq!(script, "echo $'foo bar' > $'/path/(to)/[output]'"); ``` diff --git a/src/lib.rs b/src/lib.rs index c135ea1..4a19a6a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -36,7 +36,7 @@ pub type Zsh = bash::Bash; /// Extension trait for pushing shell quoted byte slices, e.g. `&[u8]`, [`&str`] /// – anything that's [`Quotable`] – into byte container types like [`Vec`], -/// [`String`], [`OsString`] on Unix, and [`bstr::BString`] if it's enabled +/// [`String`], [`OsString`] on Unix, and [`bstr::BString`] if it's enabled. pub trait QuoteExt { fn push_quoted<'a, Q: Quoter, S: ?Sized + Into>>(&mut self, q: Q, s: S); } @@ -63,17 +63,6 @@ impl QuoteExt for bstr::BString { } } -impl QuoteExt for String { - fn push_quoted<'a, Q: Quoter, S: ?Sized + Into>>(&mut self, _q: Q, s: S) { - let quoted = Q::quote(s); - // SAFETY: `quoted` is valid UTF-8 (ASCII, in truth) because it was - // generated by a `quote` implementation from this crate – - // enforced because `Quoter` is sealed. - let quoted = unsafe { std::str::from_utf8_unchecked("ed) }; - self.push_str(quoted); - } -} - // ---------------------------------------------------------------------------- /// Extension trait for shell quoting many different owned and reference types, @@ -116,19 +105,6 @@ where } } -impl<'a, S> QuoteRefExt for S -where - S: ?Sized + Into>, -{ - fn quoted(self, _q: Q) -> String { - let quoted = Q::quote(self); - // SAFETY: `quoted` is valid UTF-8 (ASCII, in truth) because it was - // generated by a `quote` implementation from this crate – - // enforced because `Quoter` is sealed. - unsafe { String::from_utf8_unchecked(quoted) } - } -} - // ---------------------------------------------------------------------------- pub(crate) mod quoter { diff --git a/tests/test_bash.rs b/tests/test_bash.rs index 0eff885..83c8e48 100644 --- a/tests/test_bash.rs +++ b/tests/test_bash.rs @@ -109,7 +109,6 @@ mod bash_impl { // -- QuoteExt ---------------------------------------------------------------- mod bash_quote_ext { - use super::util::{find_bins, invoke_shell}; use shell_quote::{Bash, QuoteExt}; #[test] @@ -131,23 +130,28 @@ mod bash_quote_ext { assert_eq!(string, "Hello, $'World, Bob, !@#$%^&*(){}[]'"); } + #[cfg(feature = "bstr")] #[test] - fn test_string_push_quoted_with_bash() { - let mut string: String = "Hello, ".into(); + fn test_bstring_push_quoted_with_bash() { + let mut string: bstr::BString = "Hello, ".into(); string.push_quoted(Bash, "World, Bob, !@#$%^&*(){}[]"); assert_eq!(string, "Hello, $'World, Bob, !@#$%^&*(){}[]'"); } + #[cfg(feature = "bstr")] #[test] - fn test_string_push_quoted_roundtrip() { - let mut script = "printf %s ".to_owned(); + fn test_bstring_push_quoted_roundtrip() { + use super::util::{find_bins, invoke_shell}; + use bstr::{BString, ByteSlice}; + let mut script: BString = "printf %s ".into(); // It doesn't seem possible to roundtrip NUL, probably because it is the // string terminator character in C. let string: Vec<_> = (1u8..=u8::MAX).collect(); script.push_quoted(Bash, &string); + let script = script.to_os_str().unwrap(); // Test with every version of `bash` we find on `PATH`. for bin in find_bins("bash") { - let output = invoke_shell(&bin, script.as_ref()).unwrap(); + let output = invoke_shell(&bin, script).unwrap(); assert_eq!(output.stdout, string); } } diff --git a/tests/test_fish.rs b/tests/test_fish.rs index e77052f..9e24c3c 100644 --- a/tests/test_fish.rs +++ b/tests/test_fish.rs @@ -106,7 +106,6 @@ mod fish_impl { // -- QuoteExt ---------------------------------------------------------------- mod fish_quote_ext { - use super::util::{find_bins, invoke_shell}; use shell_quote::{Fish, QuoteExt}; #[test] @@ -128,25 +127,30 @@ mod fish_quote_ext { assert_eq!(string, "Hello, World,' Bob, !@#$%^&*(){}[]'"); } + #[cfg(feature = "bstr")] #[test] - fn test_string_push_quoted_with_fish() { - let mut string: String = "Hello, ".into(); + fn test_bstring_push_quoted_with_fish() { + let mut string: bstr::BString = "Hello, ".into(); string.push_quoted(Fish, "World, Bob, !@#$%^&*(){}[]"); assert_eq!(string, "Hello, World,' Bob, !@#$%^&*(){}[]'"); } + #[cfg(feature = "bstr")] #[test] - fn test_string_push_quoted_roundtrip() { + fn test_bstring_push_quoted_roundtrip() { + use super::util::{find_bins, invoke_shell}; + use bstr::{BString, ByteSlice}; // Unlike many/most other shells, `echo` is safe here because backslash // escapes are _not_ interpreted by default. - let mut script = "echo -n -- ".to_owned(); + let mut script: BString = "echo -n -- ".into(); // It doesn't seem possible to roundtrip NUL, probably because it is the // string terminator character in C. let string: Vec<_> = (1u8..=u8::MAX).collect(); script.push_quoted(Fish, &string); + let script = script.to_os_str().unwrap(); // Test with every version of `fish` we find on `PATH`. for bin in find_bins("fish") { - let output = invoke_shell(&bin, script.as_ref()).unwrap(); + let output = invoke_shell(&bin, script).unwrap(); assert_eq!(output.stdout, string); } } diff --git a/tests/test_sh.rs b/tests/test_sh.rs index e0ac865..061a8b9 100644 --- a/tests/test_sh.rs +++ b/tests/test_sh.rs @@ -179,7 +179,6 @@ mod sh_impl { // -- QuoteExt ---------------------------------------------------------------- mod sh_quote_ext { - use super::util::{find_bins, invoke_shell}; use shell_quote::{QuoteExt, Sh}; #[test] @@ -201,23 +200,28 @@ mod sh_quote_ext { assert_eq!(string, "Hello, World,' Bob, !@#$%^&*(){}[]'"); } + #[cfg(feature = "bstr")] #[test] - fn test_string_push_quoted() { - let mut string: String = "Hello, ".into(); + fn test_bstring_push_quoted() { + let mut string: bstr::BString = "Hello, ".into(); string.push_quoted(Sh, "World, Bob, !@#$%^&*(){}[]"); assert_eq!(string, "Hello, World,' Bob, !@#$%^&*(){}[]'"); } + #[cfg(feature = "bstr")] #[test] - fn test_string_push_quoted_roundtrip() { - let mut script = "printf %s ".to_owned(); + fn test_bstring_push_quoted_roundtrip() { + use super::util::{find_bins, invoke_shell}; + use bstr::{BString, ByteSlice}; + let mut script: BString = "printf %s ".into(); // It doesn't seem possible to roundtrip NUL, probably because it is the // string terminator character in C. let string: Vec<_> = (1..=u8::MAX).collect(); script.push_quoted(Sh, &string); + let script = script.to_os_str().unwrap(); // Test with every version of `sh` we find on `PATH`. for bin in find_bins("sh") { - let output = invoke_shell(&bin, script.as_ref()).unwrap(); + let output = invoke_shell(&bin, script).unwrap(); assert_eq!(output.stdout, string); } } diff --git a/tests/test_ux.rs b/tests/test_ux.rs index 0d74339..47c5a99 100644 --- a/tests/test_ux.rs +++ b/tests/test_ux.rs @@ -49,8 +49,6 @@ fn test_quote_ref_ext_byte_array() { assert_eq!(OsString::from_vec(b"$'bytes!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'bytes!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'bytes!'"), quoted); } #[test] @@ -62,8 +60,6 @@ fn test_quote_ref_ext_byte_slice() { assert_eq!(OsString::from_vec(b"$'bytes!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'bytes!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'bytes!'"), quoted); } #[test] @@ -75,8 +71,6 @@ fn test_quote_ref_ext_vec() { assert_eq!(OsString::from_vec(b"$'vec!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'vec!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'vec!'"), quoted); } #[test] @@ -88,8 +82,6 @@ fn test_quote_ref_ext_os_string() { assert_eq!(OsString::from_vec(b"$'os-string!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'os-string!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'os-string!'"), quoted); } #[test] @@ -102,8 +94,6 @@ fn test_quote_ref_ext_os_str() { assert_eq!(OsString::from_vec(b"$'os-str!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'os-str!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'os-str!'"), quoted); } #[test] @@ -115,8 +105,6 @@ fn test_quote_ref_ext_b_string() { assert_eq!(OsString::from_vec(b"$'b-string!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'b-string!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'b-string!'"), quoted); } #[test] @@ -129,8 +117,6 @@ fn test_quote_ref_ext_b_str() { assert_eq!(OsString::from_vec(b"$'b-str!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'b-str!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'b-str!'"), quoted); } #[test] @@ -142,8 +128,6 @@ fn test_quote_ref_ext_path_buf() { assert_eq!(OsString::from_vec(b"$'path-buf!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'path-buf!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'path-buf!'"), quoted); } #[test] @@ -156,8 +140,6 @@ fn test_quote_ref_ext_path() { assert_eq!(OsString::from_vec(b"$'path!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'path!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'path!'"), quoted); } #[test] @@ -169,8 +151,6 @@ fn test_quote_ref_ext_string() { assert_eq!(OsString::from_vec(b"$'string!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'string!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'string!'"), quoted); } #[test] @@ -182,6 +162,4 @@ fn test_quote_ref_ext_str() { assert_eq!(OsString::from_vec(b"$'str!'".into()), quoted); let quoted: BString = source.quoted(Bash); assert_eq!(BString::from(b"$'str!'"), quoted); - let quoted: String = source.quoted(Bash); - assert_eq!(String::from("$'str!'"), quoted); }