From 28e844500c61983cc745810d5c7fbab5b86d87e0 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Sun, 5 Feb 2023 19:41:55 -0600 Subject: [PATCH 01/17] Add an implementation of escape for OsStr Signed-off-by: Aalekh Patel --- src/lib.rs | 144 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 120 insertions(+), 24 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 65a2142..5e229ed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,7 @@ use std::borrow::Cow; use std::env; +use std::ffi::OsStr; /// Escape characters that may have special meaning in a shell. pub fn escape(s: Cow) -> Cow { @@ -22,6 +23,16 @@ pub fn escape(s: Cow) -> Cow { } } +/// Escape characters that may have special meaning in a shell +/// for an `OsStr`. +pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { + if cfg!(unix) || env::var("MSYSTEM").is_ok() { + unix::escape_os_str(s) + } else { + unimplemented!("windows::escape_os_str") + } +} + /// Windows-specific escaping. pub mod windows { use std::borrow::Cow; @@ -73,26 +84,36 @@ pub mod windows { es.into() } - #[test] - fn test_escape() { - assert_eq!(escape("--aaa=bbb-ccc".into()), "--aaa=bbb-ccc"); - assert_eq!(escape("linker=gcc -L/foo -Wl,bar".into()), - r#""linker=gcc -L/foo -Wl,bar""#); - assert_eq!(escape(r#"--features="default""#.into()), - r#""--features=\"default\"""#); - assert_eq!(escape(r#"\path\to\my documents\"#.into()), - r#""\path\to\my documents\\""#); - assert_eq!(escape("".into()), r#""""#); + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn test_escape() { + assert_eq!(escape("--aaa=bbb-ccc".into()), "--aaa=bbb-ccc"); + assert_eq!(escape("linker=gcc -L/foo -Wl,bar".into()), + r#""linker=gcc -L/foo -Wl,bar""#); + assert_eq!(escape(r#"--features="default""#.into()), + r#""--features=\"default\"""#); + assert_eq!(escape(r#"\path\to\my documents\"#.into()), + r#""\path\to\my documents\\""#); + assert_eq!(escape("".into()), r#""""#); + } } } /// Unix-specific escaping. pub mod unix { - use std::borrow::Cow; + use std::{ + borrow::Cow, + ffi::{OsStr, OsString}, + os::unix::ffi::OsStrExt, + os::unix::ffi::OsStringExt, + }; fn non_whitelisted(ch: char) -> bool { match ch { - 'a'...'z' | 'A'...'Z' | '0'...'9' | '-' | '_' | '=' | '/' | ',' | '.' | '+' => false, + 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '=' | '/' | ',' | '.' | '+' => false, _ => true, } } @@ -119,17 +140,92 @@ pub mod unix { es.into() } - #[test] - fn test_escape() { - assert_eq!( - escape("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+".into()), - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" - ); - assert_eq!(escape("--aaa=bbb-ccc".into()), "--aaa=bbb-ccc"); - assert_eq!(escape("linker=gcc -L/foo -Wl,bar".into()), r#"'linker=gcc -L/foo -Wl,bar'"#); - assert_eq!(escape(r#"--features="default""#.into()), r#"'--features="default"'"#); - assert_eq!(escape(r#"'!\$`\\\n "#.into()), r#"''\'''\!'\$`\\\n '"#); - assert_eq!(escape("".into()), r#"''"#); + #[cfg(test)] + mod tests { + use super::{ + escape, + escape_os_str + }; + use std::os::unix::ffi::OsStrExt; + use std::ffi::OsStr; + + #[test] + fn test_escape() { + assert_eq!( + escape("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+".into()), + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" + ); + assert_eq!(escape("--aaa=bbb-ccc".into()), "--aaa=bbb-ccc"); + assert_eq!(escape("linker=gcc -L/foo -Wl,bar".into()), r#"'linker=gcc -L/foo -Wl,bar'"#); + assert_eq!(escape(r#"--features="default""#.into()), r#"'--features="default"'"#); + assert_eq!(escape(r#"'!\$`\\\n "#.into()), r#"''\'''\!'\$`\\\n '"#); + assert_eq!(escape("".into()), r#"''"#); + } + + fn test_escape_os_str_case(input: &str, expected: &str) { + test_escape_os_str_from_bytes(input.as_bytes(), expected.as_bytes()) + } + + fn test_escape_os_str_from_bytes(input: &[u8], expected: &[u8]) { + let input_os_str = OsStr::from_bytes(input); + let observed_os_str = escape_os_str(input_os_str); + let expected_os_str = OsStr::from_bytes(expected); + assert_eq!(observed_os_str, expected_os_str); + } + + #[test] + fn test_escape_os_str() { + test_escape_os_str_case( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", + ); + test_escape_os_str_case("--aaa=bbb-ccc", "--aaa=bbb-ccc"); + test_escape_os_str_case( + "linker=gcc -L/foo -Wl,bar", + r#"'linker=gcc -L/foo -Wl,bar'"#, + ); + test_escape_os_str_case(r#"--features="default""#, r#"'--features="default"'"#); + test_escape_os_str_case(r#"'!\$`\\\n "#, r#"''\'''\!'\$`\\\n '"#); + test_escape_os_str_case("", r#"''"#); + test_escape_os_str_case(" ", r#"' '"#); + + test_escape_os_str_from_bytes( + &[0x66, 0x6f, 0x80, 0x6f], + &[b'\'', 0x66, 0x6f, 0x80, 0x6f, b'\''], + ); + } + } + + fn allowed(byte: u8) -> bool { + matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'-' | b'_' | b'=' | b'/' | b',' | b'.' | b'+') } -} + /// Escape characters that may have special meaning in a shell, including spaces. + /// Work with `OsStr` instead of `str`. + pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { + let as_bytes = s.as_bytes(); + let all_whitelisted = as_bytes.iter().copied().all(allowed); + + if !as_bytes.is_empty() && all_whitelisted { + return Cow::Borrowed(s); + } + + let mut escaped = Vec::with_capacity(as_bytes.len() + 2); + escaped.push(b'\''); + + for &b in as_bytes { + match b { + b'\'' | b'!' => { + escaped.reserve(4); + escaped.push(b'\''); + escaped.push(b'\\'); + escaped.push(b); + escaped.push(b'\''); + } + _ => escaped.push(b), + } + } + escaped.push(b'\''); + OsString::from_vec(escaped).into() + } +} From a32705db6f4a8d7d8c6d2cbc70d4bc02909a3660 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Sun, 5 Feb 2023 19:44:10 -0600 Subject: [PATCH 02/17] Clippy fix. Signed-off-by: Aalekh Patel --- src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5e229ed..1d246fe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -112,10 +112,7 @@ pub mod unix { }; fn non_whitelisted(ch: char) -> bool { - match ch { - 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '=' | '/' | ',' | '.' | '+' => false, - _ => true, - } + !matches!(ch, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '=' | '/' | ',' | '.' | '+') } /// Escape characters that may have special meaning in a shell, including spaces. From 7d5ca97bd7e3b0622a0d9a91a98532d1ddf32e45 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Sun, 5 Feb 2023 19:44:31 -0600 Subject: [PATCH 03/17] Rustfmt fix. Signed-off-by: Aalekh Patel --- src/lib.rs | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1d246fe..79c13f0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. //! Escape characters that may have special meaning in a shell. -#![doc(html_root_url="https://docs.rs/shell-escape/0.1")] +#![doc(html_root_url = "https://docs.rs/shell-escape/0.1")] use std::borrow::Cow; use std::env; @@ -52,7 +52,7 @@ pub mod windows { } } if !needs_escape { - return s + return s; } let mut es = String::with_capacity(s.len()); es.push('"'); @@ -78,7 +78,6 @@ pub mod windows { break; } } - } es.push('"'); es.into() @@ -91,12 +90,18 @@ pub mod windows { #[test] fn test_escape() { assert_eq!(escape("--aaa=bbb-ccc".into()), "--aaa=bbb-ccc"); - assert_eq!(escape("linker=gcc -L/foo -Wl,bar".into()), - r#""linker=gcc -L/foo -Wl,bar""#); - assert_eq!(escape(r#"--features="default""#.into()), - r#""--features=\"default\"""#); - assert_eq!(escape(r#"\path\to\my documents\"#.into()), - r#""\path\to\my documents\\""#); + assert_eq!( + escape("linker=gcc -L/foo -Wl,bar".into()), + r#""linker=gcc -L/foo -Wl,bar""# + ); + assert_eq!( + escape(r#"--features="default""#.into()), + r#""--features=\"default\"""# + ); + assert_eq!( + escape(r#"\path\to\my documents\"#.into()), + r#""\path\to\my documents\\""# + ); assert_eq!(escape("".into()), r#""""#); } } @@ -139,22 +144,27 @@ pub mod unix { #[cfg(test)] mod tests { - use super::{ - escape, - escape_os_str - }; - use std::os::unix::ffi::OsStrExt; + use super::{escape, escape_os_str}; use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; #[test] fn test_escape() { assert_eq!( - escape("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+".into()), + escape( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+".into() + ), "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" ); assert_eq!(escape("--aaa=bbb-ccc".into()), "--aaa=bbb-ccc"); - assert_eq!(escape("linker=gcc -L/foo -Wl,bar".into()), r#"'linker=gcc -L/foo -Wl,bar'"#); - assert_eq!(escape(r#"--features="default""#.into()), r#"'--features="default"'"#); + assert_eq!( + escape("linker=gcc -L/foo -Wl,bar".into()), + r#"'linker=gcc -L/foo -Wl,bar'"# + ); + assert_eq!( + escape(r#"--features="default""#.into()), + r#"'--features="default"'"# + ); assert_eq!(escape(r#"'!\$`\\\n "#.into()), r#"''\'''\!'\$`\\\n '"#); assert_eq!(escape("".into()), r#"''"#); } From ebef84ff7f88dbba76614c691c1250cd058194e2 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Sun, 5 Feb 2023 19:49:31 -0600 Subject: [PATCH 04/17] Reorder the implementation to be above tests. Signed-off-by: Aalekh Patel --- src/lib.rs | 64 +++++++++++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 79c13f0..5768534 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -142,6 +142,38 @@ pub mod unix { es.into() } + fn allowed(byte: u8) -> bool { + matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'-' | b'_' | b'=' | b'/' | b',' | b'.' | b'+') + } + + /// Escape characters that may have special meaning in a shell, including spaces. + /// Work with `OsStr` instead of `str`. + pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { + let as_bytes = s.as_bytes(); + let all_whitelisted = as_bytes.iter().copied().all(allowed); + + if !as_bytes.is_empty() && all_whitelisted { + return Cow::Borrowed(s); + } + + let mut escaped = Vec::with_capacity(as_bytes.len() + 2); + escaped.push(b'\''); + + for &b in as_bytes { + match b { + b'\'' | b'!' => { + escaped.reserve(4); + escaped.push(b'\''); + escaped.push(b'\\'); + escaped.push(b); + escaped.push(b'\''); + } + _ => escaped.push(b), + } + } + escaped.push(b'\''); + OsString::from_vec(escaped).into() + } #[cfg(test)] mod tests { use super::{escape, escape_os_str}; @@ -203,36 +235,4 @@ pub mod unix { } } - fn allowed(byte: u8) -> bool { - matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'-' | b'_' | b'=' | b'/' | b',' | b'.' | b'+') - } - - /// Escape characters that may have special meaning in a shell, including spaces. - /// Work with `OsStr` instead of `str`. - pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { - let as_bytes = s.as_bytes(); - let all_whitelisted = as_bytes.iter().copied().all(allowed); - - if !as_bytes.is_empty() && all_whitelisted { - return Cow::Borrowed(s); - } - - let mut escaped = Vec::with_capacity(as_bytes.len() + 2); - escaped.push(b'\''); - - for &b in as_bytes { - match b { - b'\'' | b'!' => { - escaped.reserve(4); - escaped.push(b'\''); - escaped.push(b'\\'); - escaped.push(b); - escaped.push(b'\''); - } - _ => escaped.push(b), - } - } - escaped.push(b'\''); - OsString::from_vec(escaped).into() - } } From 143f0c31c3a1a421a17d714f628efd0d24598930 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Sat, 11 Feb 2023 15:42:24 -0600 Subject: [PATCH 05/17] Make escape_os_str tests more readable. Signed-off-by: Aalekh Patel --- Cargo.toml | 3 +++ src/lib.rs | 71 ++++++++++++++++++++++++++++++++++++------------------ 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f78d51a..0a9c913 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,3 +7,6 @@ repository = "https://github.com/sfackler/shell-escape" description = "Escape characters that may have a special meaning in a shell" [dependencies] + +[dev-dependencies] +test-case = "2.2.2" diff --git a/src/lib.rs b/src/lib.rs index 5768534..836330d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -176,6 +176,8 @@ pub mod unix { } #[cfg(test)] mod tests { + extern crate test_case; + use super::{escape, escape_os_str}; use std::ffi::OsStr; use std::os::unix::ffi::OsStrExt; @@ -201,38 +203,59 @@ pub mod unix { assert_eq!(escape("".into()), r#"''"#); } - fn test_escape_os_str_case(input: &str, expected: &str) { - test_escape_os_str_from_bytes(input.as_bytes(), expected.as_bytes()) + #[test_case::test_case( + " ", + r#"' '"# + ; "Space is escaped by wrapping it in single quotes." + )] + #[test_case::test_case( + "", + r#"''"# + ; "Empty string is escaped by wrapping it in single quotes." + )] + #[test_case::test_case( + r#"'!\$`\\\n "#, + r#"''\'''\!'\$`\\\n '"# + ; "Text with a mix of characters that require escaping are individually escaped as well as wrapping the whole thing in single quotes." + )] + #[test_case::test_case( + r#"--features="default""#, + r#"'--features="default"'"# + ; "Text with a double quote is escaped by wrapping it all in single quotes." + )] + #[test_case::test_case( + "linker=gcc -L/foo -Wl,bar", + r#"'linker=gcc -L/foo -Wl,bar'"# + ; "Text with a slash is escaped by wrapping it all in single quotes." + )] + #[test_case::test_case( + "--aaa=bbb-ccc", + "--aaa=bbb-ccc" + ; "a flag built up entirely of allowed characters is not escaped." + )] + #[test_case::test_case( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" + ; "all allowed characters that do not require escaping are not escaped" + )] + fn test_escape_os_str(input: &str, expected: &str) { + let input_os_str = OsStr::from_bytes(input.as_bytes()); + let observed_os_str = escape_os_str(input_os_str); + let expected_os_str = OsStr::from_bytes(expected.as_bytes()); + assert_eq!(observed_os_str, expected_os_str); } + #[test_case::test_case( + &[0x66, 0x6f, 0x80, 0x6f], + &[b'\'', 0x66, 0x6f, 0x80, 0x6f, b'\''] + ; "Bytes that are not valid UTF-8 are escaped by wrapping them in single quotes." + )] fn test_escape_os_str_from_bytes(input: &[u8], expected: &[u8]) { let input_os_str = OsStr::from_bytes(input); let observed_os_str = escape_os_str(input_os_str); let expected_os_str = OsStr::from_bytes(expected); assert_eq!(observed_os_str, expected_os_str); } - - #[test] - fn test_escape_os_str() { - test_escape_os_str_case( - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", - ); - test_escape_os_str_case("--aaa=bbb-ccc", "--aaa=bbb-ccc"); - test_escape_os_str_case( - "linker=gcc -L/foo -Wl,bar", - r#"'linker=gcc -L/foo -Wl,bar'"#, - ); - test_escape_os_str_case(r#"--features="default""#, r#"'--features="default"'"#); - test_escape_os_str_case(r#"'!\$`\\\n "#, r#"''\'''\!'\$`\\\n '"#); - test_escape_os_str_case("", r#"''"#); - test_escape_os_str_case(" ", r#"' '"#); - - test_escape_os_str_from_bytes( - &[0x66, 0x6f, 0x80, 0x6f], - &[b'\'', 0x66, 0x6f, 0x80, 0x6f, b'\''], - ); - } } } From 791a4aafa22a1a1f46a4b51ec67dc05662caff11 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Sat, 11 Feb 2023 16:05:43 -0600 Subject: [PATCH 06/17] Make escape tests more readable. Signed-off-by: Aalekh Patel --- src/lib.rs | 56 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 836330d..a69e015 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -182,25 +182,43 @@ pub mod unix { use std::ffi::OsStr; use std::os::unix::ffi::OsStrExt; - #[test] - fn test_escape() { - assert_eq!( - escape( - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+".into() - ), - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" - ); - assert_eq!(escape("--aaa=bbb-ccc".into()), "--aaa=bbb-ccc"); - assert_eq!( - escape("linker=gcc -L/foo -Wl,bar".into()), - r#"'linker=gcc -L/foo -Wl,bar'"# - ); - assert_eq!( - escape(r#"--features="default""#.into()), - r#"'--features="default"'"# - ); - assert_eq!(escape(r#"'!\$`\\\n "#.into()), r#"''\'''\!'\$`\\\n '"#); - assert_eq!(escape("".into()), r#"''"#); + #[test_case::test_case( + " ", + r#"' '"# + ; "Space is escaped by wrapping it in single quotes." + )] + #[test_case::test_case( + "", + r#"''"# + ; "Empty string is escaped by wrapping it in single quotes." + )] + #[test_case::test_case( + r#"'!\$`\\\n "#, + r#"''\'''\!'\$`\\\n '"# + ; "Text with a mix of characters that require escaping are individually escaped as well as wrapping the whole thing in single quotes." + )] + #[test_case::test_case( + r#"--features="default""#, + r#"'--features="default"'"# + ; "Text with a double quote is escaped by wrapping it all in single quotes." + )] + #[test_case::test_case( + "linker=gcc -L/foo -Wl,bar", + r#"'linker=gcc -L/foo -Wl,bar'"# + ; "Text with a slash is escaped by wrapping it all in single quotes." + )] + #[test_case::test_case( + "--aaa=bbb-ccc", + "--aaa=bbb-ccc" + ; "a flag built up entirely of allowed characters is not escaped." + )] + #[test_case::test_case( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" + ; "all allowed characters that do not require escaping are not escaped" + )] + fn test_escape(input: &str, expected: &str) { + assert_eq!(escape(input.into()), expected); } #[test_case::test_case( From 3b964b4c3837636340a3a4aa0134eb87b0801297 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Sat, 11 Feb 2023 16:10:32 -0600 Subject: [PATCH 07/17] Use test_case in windows escape tests as well. Signed-off-by: Aalekh Patel --- src/lib.rs | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a69e015..c755083 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -86,23 +86,35 @@ pub mod windows { #[cfg(test)] mod tests { use super::*; + extern crate test_case; - #[test] - fn test_escape() { - assert_eq!(escape("--aaa=bbb-ccc".into()), "--aaa=bbb-ccc"); - assert_eq!( - escape("linker=gcc -L/foo -Wl,bar".into()), - r#""linker=gcc -L/foo -Wl,bar""# - ); - assert_eq!( - escape(r#"--features="default""#.into()), - r#""--features=\"default\"""# - ); - assert_eq!( - escape(r#"\path\to\my documents\"#.into()), - r#""\path\to\my documents\\""# - ); - assert_eq!(escape("".into()), r#""""#); + #[test_case::test_case( + r#""""#, + r#""\"\"""# + ; "an empty string is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#""--features=\"default\"""#, + r#""\"--features=\\\"default\\\"\"""# + ; "a flag with quotes is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#"linker=gcc -L/foo -Wl,bar"#, + r#""linker=gcc -L/foo -Wl,bar""# + ; "a flag with spaces is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#"\path\to\my documents\"#, + r#""\path\to\my documents\\""# + ; "a path with spaces is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + "--aaa=bbb-ccc", + "--aaa=bbb-ccc" + ; "a flag built up entirely of allowed characters is not escaped." + )] + fn test_escape(input: &str, expected: &str) { + assert_eq!(escape(input.into()), expected); } } } @@ -275,5 +287,4 @@ pub mod unix { assert_eq!(observed_os_str, expected_os_str); } } - } From ae6c50cbc753cf4ad72fee4640a479549f359c92 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Mon, 13 Feb 2023 21:36:15 -0600 Subject: [PATCH 08/17] Add an `escape_os_str` for Windows. --- src/lib.rs | 185 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 162 insertions(+), 23 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c755083..9d86744 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,34 +10,33 @@ //! Escape characters that may have special meaning in a shell. #![doc(html_root_url = "https://docs.rs/shell-escape/0.1")] -use std::borrow::Cow; -use std::env; -use std::ffi::OsStr; - -/// Escape characters that may have special meaning in a shell. -pub fn escape(s: Cow) -> Cow { - if cfg!(unix) || env::var("MSYSTEM").is_ok() { - unix::escape(s) - } else { - windows::escape(s) - } -} -/// Escape characters that may have special meaning in a shell -/// for an `OsStr`. -pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { - if cfg!(unix) || env::var("MSYSTEM").is_ok() { - unix::escape_os_str(s) - } else { - unimplemented!("windows::escape_os_str") - } -} +#[cfg(unix)] +pub use unix::escape_os_str as escape_os_str; +#[cfg(unix)] +pub use unix::escape as escape; + +#[cfg(windows)] +pub use windows::escape as escape; +#[cfg(windows)] +pub use windows::escape_os_str as escape_os_str; + +#[cfg(windows)] /// Windows-specific escaping. pub mod windows { use std::borrow::Cow; + use std::ffi::{ + OsStr, + OsString + }; use std::iter::repeat; + use std::os::windows::ffi::{ + OsStrExt, + OsStringExt + }; + /// Escape for the windows cmd.exe shell. /// /// See [here][msdn] for more information. @@ -83,6 +82,92 @@ pub mod windows { es.into() } + + /// Determine if a wide byte needs to be escaped. + /// Only tabs, newlines, spaces, and double quotes need to be escaped. + /// Example: + /// ``` + /// use shell_escape::windows::needs_escape; + /// + /// assert!(needs_escape(b'"' as u16)); + /// assert!(needs_escape(b'\t' as u16)); + /// assert!(needs_escape(b'\n' as u16)); + /// assert!(needs_escape(b' ' as u16)); + /// assert!(!needs_escape(b'\\' as u16)); + /// ``` + pub fn needs_escape(wide_byte: u16) -> bool { + + let (high, low) = ((wide_byte >> 8) as u8, (wide_byte & 0xFF) as u8); + + if high > 0 { + // High byte is set, so its not an ASCII character and definitely needs escaping. + return true; + } + matches!( + low, + b'"' | b'\t' | b'\n' | b' ' + ) + } + + /// Escape OsStr for the windows cmd.exe shell. + /// + /// See [here][msdn] for more information. + /// + /// Example: + /// ``` + /// use shell_escape::windows::escape_os_str; + /// use std::ffi::OsStr; + /// + /// let s = OsStr::new("foo bar"); + /// let escaped = escape_os_str(s); + /// assert_eq!(escaped, OsStr::new(r#""foo bar""#)); + /// ``` + /// [msdn]: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx + pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { + let encoded: Vec = s.encode_wide().collect(); + let needs_escaping = encoded.iter().copied().any(needs_escape); + + if s.is_empty() || !needs_escaping { + return Cow::Borrowed(s); + } + + let mut escaped = Vec::with_capacity(encoded.len() + 2); + escaped.push(b'"' as u16); + + let mut chars = encoded.into_iter().peekable(); + loop { + let mut nslashes = 0; + while let Some(&c) = chars.peek() { + if c == (b'\\' as u16) { + chars.next(); + nslashes += 1; + } else { + break; + } + } + match chars.next() { + Some(c) if c == b'"' as u16 => { + escaped.reserve(nslashes * 2 + 1); + escaped.extend(repeat(b'\\' as u16).take(nslashes * 2 + 1)); + escaped.push(b'"' as u16); + }, + Some(c) => { + escaped.reserve(nslashes); + escaped.extend(repeat(b'\\' as u16).take(nslashes)); + escaped.push(c); + }, + None => { + escaped.reserve(nslashes * 2); + escaped.extend(repeat(b'\\' as u16).take(nslashes * 2)); + break; + } + } + } + + escaped.push(b'"' as u16); + OsString::from_wide(&escaped).into() + } + #[cfg(test)] mod tests { use super::*; @@ -116,16 +201,69 @@ pub mod windows { fn test_escape(input: &str, expected: &str) { assert_eq!(escape(input.into()), expected); } + + #[test_case::test_case( + r#""""#, + r#""\"\"""# + ; "an empty string is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#""--features=\"default\"""#, + r#""\"--features=\\\"default\\\"\"""# + ; "a flag with quotes is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#"linker=gcc -L/foo -Wl,bar"#, + r#""linker=gcc -L/foo -Wl,bar""# + ; "a flag with spaces is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#"\path\to\my documents\"#, + r#""\path\to\my documents\\""# + ; "a path with spaces is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + "--aaa=bbb-ccc", + "--aaa=bbb-ccc" + ; "a flag built up entirely of allowed characters is not escaped." + )] + fn test_escape_os_str(input: &str, expected: &str) { + let binding = OsString::from(input); + let input_os_str = binding.as_os_str(); + let binding = OsString::from(expected); + let expected_os_str = binding.as_os_str(); + let observed_os_str = escape_os_str(input_os_str); + assert_eq!(observed_os_str, expected_os_str); + } + + /// FIXME: Need to fix this test case. + /// I'm not sure what we're expecting to happen here. + #[test_case::test_case( + &[0x1055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065], + &[0x1055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065] + ; "A u16 with high byte set requires escaping." + )] + fn test_escape_os_str_from_bytes(input: &[u16], expected: &[u16]) { + let binding = OsString::from_wide(input); + let input_os_str = binding.as_os_str(); + let binding = OsString::from_wide(expected); + let expected_os_str = binding.as_os_str(); + let observed_os_str = escape_os_str(input_os_str); + assert_eq!(observed_os_str, expected_os_str); + } } } /// Unix-specific escaping. +#[cfg(unix)] pub mod unix { use std::{ borrow::Cow, ffi::{OsStr, OsString}, - os::unix::ffi::OsStrExt, - os::unix::ffi::OsStringExt, + os::unix::ffi::{ + OsStrExt, + OsStringExt + } }; fn non_whitelisted(ch: char) -> bool { @@ -186,6 +324,7 @@ pub mod unix { escaped.push(b'\''); OsString::from_vec(escaped).into() } + #[cfg(test)] mod tests { extern crate test_case; From 4e148e27d2e328e0d880b18a39eb4f28470b6b85 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Mon, 13 Feb 2023 21:37:26 -0600 Subject: [PATCH 09/17] Apply rustfmt fixes. --- src/lib.rs | 42 +++++++++++++----------------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9d86744..a1d2e98 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,32 +10,24 @@ //! Escape characters that may have special meaning in a shell. #![doc(html_root_url = "https://docs.rs/shell-escape/0.1")] - #[cfg(unix)] -pub use unix::escape_os_str as escape_os_str; +pub use unix::escape; #[cfg(unix)] -pub use unix::escape as escape; +pub use unix::escape_os_str; #[cfg(windows)] -pub use windows::escape as escape; +pub use windows::escape; #[cfg(windows)] -pub use windows::escape_os_str as escape_os_str; - +pub use windows::escape_os_str; #[cfg(windows)] /// Windows-specific escaping. pub mod windows { use std::borrow::Cow; - use std::ffi::{ - OsStr, - OsString - }; + use std::ffi::{OsStr, OsString}; use std::iter::repeat; - use std::os::windows::ffi::{ - OsStrExt, - OsStringExt - }; + use std::os::windows::ffi::{OsStrExt, OsStringExt}; /// Escape for the windows cmd.exe shell. /// @@ -82,13 +74,12 @@ pub mod windows { es.into() } - /// Determine if a wide byte needs to be escaped. /// Only tabs, newlines, spaces, and double quotes need to be escaped. /// Example: /// ``` /// use shell_escape::windows::needs_escape; - /// + /// /// assert!(needs_escape(b'"' as u16)); /// assert!(needs_escape(b'\t' as u16)); /// assert!(needs_escape(b'\n' as u16)); @@ -96,17 +87,13 @@ pub mod windows { /// assert!(!needs_escape(b'\\' as u16)); /// ``` pub fn needs_escape(wide_byte: u16) -> bool { - let (high, low) = ((wide_byte >> 8) as u8, (wide_byte & 0xFF) as u8); if high > 0 { // High byte is set, so its not an ASCII character and definitely needs escaping. return true; } - matches!( - low, - b'"' | b'\t' | b'\n' | b' ' - ) + matches!(low, b'"' | b'\t' | b'\n' | b' ') } /// Escape OsStr for the windows cmd.exe shell. @@ -117,7 +104,7 @@ pub mod windows { /// ``` /// use shell_escape::windows::escape_os_str; /// use std::ffi::OsStr; - /// + /// /// let s = OsStr::new("foo bar"); /// let escaped = escape_os_str(s); /// assert_eq!(escaped, OsStr::new(r#""foo bar""#)); @@ -150,12 +137,12 @@ pub mod windows { escaped.reserve(nslashes * 2 + 1); escaped.extend(repeat(b'\\' as u16).take(nslashes * 2 + 1)); escaped.push(b'"' as u16); - }, + } Some(c) => { escaped.reserve(nslashes); escaped.extend(repeat(b'\\' as u16).take(nslashes)); escaped.push(c); - }, + } None => { escaped.reserve(nslashes * 2); escaped.extend(repeat(b'\\' as u16).take(nslashes * 2)); @@ -236,7 +223,7 @@ pub mod windows { assert_eq!(observed_os_str, expected_os_str); } - /// FIXME: Need to fix this test case. + /// FIXME: Need to fix this test case. /// I'm not sure what we're expecting to happen here. #[test_case::test_case( &[0x1055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065], @@ -260,10 +247,7 @@ pub mod unix { use std::{ borrow::Cow, ffi::{OsStr, OsString}, - os::unix::ffi::{ - OsStrExt, - OsStringExt - } + os::unix::ffi::{OsStrExt, OsStringExt}, }; fn non_whitelisted(ch: char) -> bool { From 934b6f11f6725064f3911ff161743c2179fe628b Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Mon, 13 Feb 2023 22:38:56 -0600 Subject: [PATCH 10/17] Refactor escape impls between two modules. --- src/lib.rs | 406 ++----------------------------------------------- src/unix.rs | 168 ++++++++++++++++++++ src/windows.rs | 217 ++++++++++++++++++++++++++ 3 files changed, 396 insertions(+), 395 deletions(-) create mode 100644 src/unix.rs create mode 100644 src/windows.rs diff --git a/src/lib.rs b/src/lib.rs index a1d2e98..f130fd2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,403 +11,19 @@ #![doc(html_root_url = "https://docs.rs/shell-escape/0.1")] #[cfg(unix)] -pub use unix::escape; +mod unix; + #[cfg(unix)] -pub use unix::escape_os_str; +pub use unix::{ + escape, + escape_os_str +}; #[cfg(windows)] -pub use windows::escape; -#[cfg(windows)] -pub use windows::escape_os_str; +mod windows; #[cfg(windows)] -/// Windows-specific escaping. -pub mod windows { - use std::borrow::Cow; - use std::ffi::{OsStr, OsString}; - use std::iter::repeat; - - use std::os::windows::ffi::{OsStrExt, OsStringExt}; - - /// Escape for the windows cmd.exe shell. - /// - /// See [here][msdn] for more information. - /// - /// [msdn]: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx - pub fn escape(s: Cow) -> Cow { - let mut needs_escape = s.is_empty(); - for ch in s.chars() { - match ch { - '"' | '\t' | '\n' | ' ' => needs_escape = true, - _ => {} - } - } - if !needs_escape { - return s; - } - let mut es = String::with_capacity(s.len()); - es.push('"'); - let mut chars = s.chars().peekable(); - loop { - let mut nslashes = 0; - while let Some(&'\\') = chars.peek() { - chars.next(); - nslashes += 1; - } - - match chars.next() { - Some('"') => { - es.extend(repeat('\\').take(nslashes * 2 + 1)); - es.push('"'); - } - Some(c) => { - es.extend(repeat('\\').take(nslashes)); - es.push(c); - } - None => { - es.extend(repeat('\\').take(nslashes * 2)); - break; - } - } - } - es.push('"'); - es.into() - } - - /// Determine if a wide byte needs to be escaped. - /// Only tabs, newlines, spaces, and double quotes need to be escaped. - /// Example: - /// ``` - /// use shell_escape::windows::needs_escape; - /// - /// assert!(needs_escape(b'"' as u16)); - /// assert!(needs_escape(b'\t' as u16)); - /// assert!(needs_escape(b'\n' as u16)); - /// assert!(needs_escape(b' ' as u16)); - /// assert!(!needs_escape(b'\\' as u16)); - /// ``` - pub fn needs_escape(wide_byte: u16) -> bool { - let (high, low) = ((wide_byte >> 8) as u8, (wide_byte & 0xFF) as u8); - - if high > 0 { - // High byte is set, so its not an ASCII character and definitely needs escaping. - return true; - } - matches!(low, b'"' | b'\t' | b'\n' | b' ') - } - - /// Escape OsStr for the windows cmd.exe shell. - /// - /// See [here][msdn] for more information. - /// - /// Example: - /// ``` - /// use shell_escape::windows::escape_os_str; - /// use std::ffi::OsStr; - /// - /// let s = OsStr::new("foo bar"); - /// let escaped = escape_os_str(s); - /// assert_eq!(escaped, OsStr::new(r#""foo bar""#)); - /// ``` - /// [msdn]: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx - pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { - let encoded: Vec = s.encode_wide().collect(); - let needs_escaping = encoded.iter().copied().any(needs_escape); - - if s.is_empty() || !needs_escaping { - return Cow::Borrowed(s); - } - - let mut escaped = Vec::with_capacity(encoded.len() + 2); - escaped.push(b'"' as u16); - - let mut chars = encoded.into_iter().peekable(); - loop { - let mut nslashes = 0; - while let Some(&c) = chars.peek() { - if c == (b'\\' as u16) { - chars.next(); - nslashes += 1; - } else { - break; - } - } - match chars.next() { - Some(c) if c == b'"' as u16 => { - escaped.reserve(nslashes * 2 + 1); - escaped.extend(repeat(b'\\' as u16).take(nslashes * 2 + 1)); - escaped.push(b'"' as u16); - } - Some(c) => { - escaped.reserve(nslashes); - escaped.extend(repeat(b'\\' as u16).take(nslashes)); - escaped.push(c); - } - None => { - escaped.reserve(nslashes * 2); - escaped.extend(repeat(b'\\' as u16).take(nslashes * 2)); - break; - } - } - } - - escaped.push(b'"' as u16); - OsString::from_wide(&escaped).into() - } - - #[cfg(test)] - mod tests { - use super::*; - extern crate test_case; - - #[test_case::test_case( - r#""""#, - r#""\"\"""# - ; "an empty string is escaped by surrounding with double quotes." - )] - #[test_case::test_case( - r#""--features=\"default\"""#, - r#""\"--features=\\\"default\\\"\"""# - ; "a flag with quotes is escaped by surrounding with double quotes." - )] - #[test_case::test_case( - r#"linker=gcc -L/foo -Wl,bar"#, - r#""linker=gcc -L/foo -Wl,bar""# - ; "a flag with spaces is escaped by surrounding with double quotes." - )] - #[test_case::test_case( - r#"\path\to\my documents\"#, - r#""\path\to\my documents\\""# - ; "a path with spaces is escaped by surrounding with double quotes." - )] - #[test_case::test_case( - "--aaa=bbb-ccc", - "--aaa=bbb-ccc" - ; "a flag built up entirely of allowed characters is not escaped." - )] - fn test_escape(input: &str, expected: &str) { - assert_eq!(escape(input.into()), expected); - } - - #[test_case::test_case( - r#""""#, - r#""\"\"""# - ; "an empty string is escaped by surrounding with double quotes." - )] - #[test_case::test_case( - r#""--features=\"default\"""#, - r#""\"--features=\\\"default\\\"\"""# - ; "a flag with quotes is escaped by surrounding with double quotes." - )] - #[test_case::test_case( - r#"linker=gcc -L/foo -Wl,bar"#, - r#""linker=gcc -L/foo -Wl,bar""# - ; "a flag with spaces is escaped by surrounding with double quotes." - )] - #[test_case::test_case( - r#"\path\to\my documents\"#, - r#""\path\to\my documents\\""# - ; "a path with spaces is escaped by surrounding with double quotes." - )] - #[test_case::test_case( - "--aaa=bbb-ccc", - "--aaa=bbb-ccc" - ; "a flag built up entirely of allowed characters is not escaped." - )] - fn test_escape_os_str(input: &str, expected: &str) { - let binding = OsString::from(input); - let input_os_str = binding.as_os_str(); - let binding = OsString::from(expected); - let expected_os_str = binding.as_os_str(); - let observed_os_str = escape_os_str(input_os_str); - assert_eq!(observed_os_str, expected_os_str); - } - - /// FIXME: Need to fix this test case. - /// I'm not sure what we're expecting to happen here. - #[test_case::test_case( - &[0x1055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065], - &[0x1055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065] - ; "A u16 with high byte set requires escaping." - )] - fn test_escape_os_str_from_bytes(input: &[u16], expected: &[u16]) { - let binding = OsString::from_wide(input); - let input_os_str = binding.as_os_str(); - let binding = OsString::from_wide(expected); - let expected_os_str = binding.as_os_str(); - let observed_os_str = escape_os_str(input_os_str); - assert_eq!(observed_os_str, expected_os_str); - } - } -} - -/// Unix-specific escaping. -#[cfg(unix)] -pub mod unix { - use std::{ - borrow::Cow, - ffi::{OsStr, OsString}, - os::unix::ffi::{OsStrExt, OsStringExt}, - }; - - fn non_whitelisted(ch: char) -> bool { - !matches!(ch, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '=' | '/' | ',' | '.' | '+') - } - - /// Escape characters that may have special meaning in a shell, including spaces. - pub fn escape(s: Cow) -> Cow { - if !s.is_empty() && !s.contains(non_whitelisted) { - return s; - } - - let mut es = String::with_capacity(s.len() + 2); - es.push('\''); - for ch in s.chars() { - match ch { - '\'' | '!' => { - es.push_str("'\\"); - es.push(ch); - es.push('\''); - } - _ => es.push(ch), - } - } - es.push('\''); - es.into() - } - - fn allowed(byte: u8) -> bool { - matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'-' | b'_' | b'=' | b'/' | b',' | b'.' | b'+') - } - - /// Escape characters that may have special meaning in a shell, including spaces. - /// Work with `OsStr` instead of `str`. - pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { - let as_bytes = s.as_bytes(); - let all_whitelisted = as_bytes.iter().copied().all(allowed); - - if !as_bytes.is_empty() && all_whitelisted { - return Cow::Borrowed(s); - } - - let mut escaped = Vec::with_capacity(as_bytes.len() + 2); - escaped.push(b'\''); - - for &b in as_bytes { - match b { - b'\'' | b'!' => { - escaped.reserve(4); - escaped.push(b'\''); - escaped.push(b'\\'); - escaped.push(b); - escaped.push(b'\''); - } - _ => escaped.push(b), - } - } - escaped.push(b'\''); - OsString::from_vec(escaped).into() - } - - #[cfg(test)] - mod tests { - extern crate test_case; - - use super::{escape, escape_os_str}; - use std::ffi::OsStr; - use std::os::unix::ffi::OsStrExt; - - #[test_case::test_case( - " ", - r#"' '"# - ; "Space is escaped by wrapping it in single quotes." - )] - #[test_case::test_case( - "", - r#"''"# - ; "Empty string is escaped by wrapping it in single quotes." - )] - #[test_case::test_case( - r#"'!\$`\\\n "#, - r#"''\'''\!'\$`\\\n '"# - ; "Text with a mix of characters that require escaping are individually escaped as well as wrapping the whole thing in single quotes." - )] - #[test_case::test_case( - r#"--features="default""#, - r#"'--features="default"'"# - ; "Text with a double quote is escaped by wrapping it all in single quotes." - )] - #[test_case::test_case( - "linker=gcc -L/foo -Wl,bar", - r#"'linker=gcc -L/foo -Wl,bar'"# - ; "Text with a slash is escaped by wrapping it all in single quotes." - )] - #[test_case::test_case( - "--aaa=bbb-ccc", - "--aaa=bbb-ccc" - ; "a flag built up entirely of allowed characters is not escaped." - )] - #[test_case::test_case( - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" - ; "all allowed characters that do not require escaping are not escaped" - )] - fn test_escape(input: &str, expected: &str) { - assert_eq!(escape(input.into()), expected); - } - - #[test_case::test_case( - " ", - r#"' '"# - ; "Space is escaped by wrapping it in single quotes." - )] - #[test_case::test_case( - "", - r#"''"# - ; "Empty string is escaped by wrapping it in single quotes." - )] - #[test_case::test_case( - r#"'!\$`\\\n "#, - r#"''\'''\!'\$`\\\n '"# - ; "Text with a mix of characters that require escaping are individually escaped as well as wrapping the whole thing in single quotes." - )] - #[test_case::test_case( - r#"--features="default""#, - r#"'--features="default"'"# - ; "Text with a double quote is escaped by wrapping it all in single quotes." - )] - #[test_case::test_case( - "linker=gcc -L/foo -Wl,bar", - r#"'linker=gcc -L/foo -Wl,bar'"# - ; "Text with a slash is escaped by wrapping it all in single quotes." - )] - #[test_case::test_case( - "--aaa=bbb-ccc", - "--aaa=bbb-ccc" - ; "a flag built up entirely of allowed characters is not escaped." - )] - #[test_case::test_case( - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" - ; "all allowed characters that do not require escaping are not escaped" - )] - fn test_escape_os_str(input: &str, expected: &str) { - let input_os_str = OsStr::from_bytes(input.as_bytes()); - let observed_os_str = escape_os_str(input_os_str); - let expected_os_str = OsStr::from_bytes(expected.as_bytes()); - assert_eq!(observed_os_str, expected_os_str); - } - - #[test_case::test_case( - &[0x66, 0x6f, 0x80, 0x6f], - &[b'\'', 0x66, 0x6f, 0x80, 0x6f, b'\''] - ; "Bytes that are not valid UTF-8 are escaped by wrapping them in single quotes." - )] - fn test_escape_os_str_from_bytes(input: &[u8], expected: &[u8]) { - let input_os_str = OsStr::from_bytes(input); - let observed_os_str = escape_os_str(input_os_str); - let expected_os_str = OsStr::from_bytes(expected); - assert_eq!(observed_os_str, expected_os_str); - } - } -} +pub use windows::{ + escape, + escape_os_str +}; diff --git a/src/unix.rs b/src/unix.rs new file mode 100644 index 0000000..e95f8df --- /dev/null +++ b/src/unix.rs @@ -0,0 +1,168 @@ +//! Unix-specific escaping. +//! +use std::{ + borrow::Cow, + ffi::{OsStr, OsString}, + os::unix::ffi::{OsStrExt, OsStringExt}, +}; + +fn non_whitelisted(ch: char) -> bool { + !matches!(ch, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '=' | '/' | ',' | '.' | '+') +} + +/// Escape characters that may have special meaning in a shell, including spaces. +pub fn escape(s: Cow) -> Cow { + if !s.is_empty() && !s.contains(non_whitelisted) { + return s; + } + + let mut es = String::with_capacity(s.len() + 2); + es.push('\''); + for ch in s.chars() { + match ch { + '\'' | '!' => { + es.push_str("'\\"); + es.push(ch); + es.push('\''); + } + _ => es.push(ch), + } + } + es.push('\''); + es.into() +} + +fn allowed(byte: u8) -> bool { + matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'-' | b'_' | b'=' | b'/' | b',' | b'.' | b'+') +} + +/// Escape characters that may have special meaning in a shell, including spaces. +/// Work with `OsStr` instead of `str`. +pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { + let as_bytes = s.as_bytes(); + let all_whitelisted = as_bytes.iter().copied().all(allowed); + + if !as_bytes.is_empty() && all_whitelisted { + return Cow::Borrowed(s); + } + + let mut escaped = Vec::with_capacity(as_bytes.len() + 2); + escaped.push(b'\''); + + for &b in as_bytes { + match b { + b'\'' | b'!' => { + escaped.reserve(4); + escaped.push(b'\''); + escaped.push(b'\\'); + escaped.push(b); + escaped.push(b'\''); + } + _ => escaped.push(b), + } + } + escaped.push(b'\''); + OsString::from_vec(escaped).into() +} + +#[cfg(test)] +mod tests { + extern crate test_case; + + use super::{escape, escape_os_str}; + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + #[test_case::test_case( + " ", + r#"' '"# + ; "Space is escaped by wrapping it in single quotes." + )] + #[test_case::test_case( + "", + r#"''"# + ; "Empty string is escaped by wrapping it in single quotes." + )] + #[test_case::test_case( + r#"'!\$`\\\n "#, + r#"''\'''\!'\$`\\\n '"# + ; "Text with a mix of characters that require escaping are individually escaped as well as wrapping the whole thing in single quotes." + )] + #[test_case::test_case( + r#"--features="default""#, + r#"'--features="default"'"# + ; "Text with a double quote is escaped by wrapping it all in single quotes." + )] + #[test_case::test_case( + "linker=gcc -L/foo -Wl,bar", + r#"'linker=gcc -L/foo -Wl,bar'"# + ; "Text with a slash is escaped by wrapping it all in single quotes." + )] + #[test_case::test_case( + "--aaa=bbb-ccc", + "--aaa=bbb-ccc" + ; "a flag built up entirely of allowed characters is not escaped." + )] + #[test_case::test_case( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" + ; "all allowed characters that do not require escaping are not escaped" + )] + fn test_escape(input: &str, expected: &str) { + assert_eq!(escape(input.into()), expected); + } + + #[test_case::test_case( + " ", + r#"' '"# + ; "Space is escaped by wrapping it in single quotes." + )] + #[test_case::test_case( + "", + r#"''"# + ; "Empty string is escaped by wrapping it in single quotes." + )] + #[test_case::test_case( + r#"'!\$`\\\n "#, + r#"''\'''\!'\$`\\\n '"# + ; "Text with a mix of characters that require escaping are individually escaped as well as wrapping the whole thing in single quotes." + )] + #[test_case::test_case( + r#"--features="default""#, + r#"'--features="default"'"# + ; "Text with a double quote is escaped by wrapping it all in single quotes." + )] + #[test_case::test_case( + "linker=gcc -L/foo -Wl,bar", + r#"'linker=gcc -L/foo -Wl,bar'"# + ; "Text with a slash is escaped by wrapping it all in single quotes." + )] + #[test_case::test_case( + "--aaa=bbb-ccc", + "--aaa=bbb-ccc" + ; "a flag built up entirely of allowed characters is not escaped." + )] + #[test_case::test_case( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+", + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_=/,.+" + ; "all allowed characters that do not require escaping are not escaped" + )] + fn test_escape_os_str(input: &str, expected: &str) { + let input_os_str = OsStr::from_bytes(input.as_bytes()); + let observed_os_str = escape_os_str(input_os_str); + let expected_os_str = OsStr::from_bytes(expected.as_bytes()); + assert_eq!(observed_os_str, expected_os_str); + } + + #[test_case::test_case( + &[0x66, 0x6f, 0x80, 0x6f], + &[b'\'', 0x66, 0x6f, 0x80, 0x6f, b'\''] + ; "Bytes that are not valid UTF-8 are escaped by wrapping them in single quotes." + )] + fn test_escape_os_str_from_bytes(input: &[u8], expected: &[u8]) { + let input_os_str = OsStr::from_bytes(input); + let observed_os_str = escape_os_str(input_os_str); + let expected_os_str = OsStr::from_bytes(expected); + assert_eq!(observed_os_str, expected_os_str); + } +} \ No newline at end of file diff --git a/src/windows.rs b/src/windows.rs new file mode 100644 index 0000000..2c9c5ea --- /dev/null +++ b/src/windows.rs @@ -0,0 +1,217 @@ +//! +//! Windows-specific escaping. +use std::borrow::Cow; +use std::ffi::{OsStr, OsString}; +use std::iter::repeat; +use std::os::windows::ffi::{OsStrExt, OsStringExt}; + +/// Escape for the windows cmd.exe shell. +/// +/// See [here][msdn] for more information. +/// +/// [msdn]: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx +pub fn escape(s: Cow) -> Cow { + let mut needs_escape = s.is_empty(); + for ch in s.chars() { + match ch { + '"' | '\t' | '\n' | ' ' => needs_escape = true, + _ => {} + } + } + if !needs_escape { + return s; + } + let mut es = String::with_capacity(s.len()); + es.push('"'); + let mut chars = s.chars().peekable(); + loop { + let mut nslashes = 0; + while let Some(&'\\') = chars.peek() { + chars.next(); + nslashes += 1; + } + + match chars.next() { + Some('"') => { + es.extend(repeat('\\').take(nslashes * 2 + 1)); + es.push('"'); + } + Some(c) => { + es.extend(repeat('\\').take(nslashes)); + es.push(c); + } + None => { + es.extend(repeat('\\').take(nslashes * 2)); + break; + } + } + } + es.push('"'); + es.into() +} + +/// Determine if a wide byte needs to be escaped. +/// Only tabs, newlines, spaces, and double quotes need to be escaped. +/// Example: +/// ``` +/// use shell_escape::windows::needs_escape; +/// +/// assert!(needs_escape(b'"' as u16)); +/// assert!(needs_escape(b'\t' as u16)); +/// assert!(needs_escape(b'\n' as u16)); +/// assert!(needs_escape(b' ' as u16)); +/// assert!(!needs_escape(b'\\' as u16)); +/// ``` +pub fn needs_escape(wide_byte: u16) -> bool { + let (high, low) = ((wide_byte >> 8) as u8, (wide_byte & 0xFF) as u8); + + if high > 0 { + // High byte is set, so its not an ASCII character and definitely needs escaping. + return true; + } + matches!(low, b'"' | b'\t' | b'\n' | b' ') +} + +/// Escape OsStr for the windows cmd.exe shell. +/// +/// See [here][msdn] for more information. +/// +/// Example: +/// ``` +/// use shell_escape::windows::escape_os_str; +/// use std::ffi::OsStr; +/// +/// let s = OsStr::new("foo bar"); +/// let escaped = escape_os_str(s); +/// assert_eq!(escaped, OsStr::new(r#""foo bar""#)); +/// ``` +/// [msdn]: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx +pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { + let encoded: Vec = s.encode_wide().collect(); + let needs_escaping = encoded.iter().copied().any(needs_escape); + + if s.is_empty() || !needs_escaping { + return Cow::Borrowed(s); + } + + let mut escaped = Vec::with_capacity(encoded.len() + 2); + escaped.push(b'"' as u16); + + let mut chars = encoded.into_iter().peekable(); + loop { + let mut nslashes = 0; + while let Some(&c) = chars.peek() { + if c == (b'\\' as u16) { + chars.next(); + nslashes += 1; + } else { + break; + } + } + match chars.next() { + Some(c) if c == b'"' as u16 => { + escaped.reserve(nslashes * 2 + 1); + escaped.extend(repeat(b'\\' as u16).take(nslashes * 2 + 1)); + escaped.push(b'"' as u16); + } + Some(c) => { + escaped.reserve(nslashes); + escaped.extend(repeat(b'\\' as u16).take(nslashes)); + escaped.push(c); + } + None => { + escaped.reserve(nslashes * 2); + escaped.extend(repeat(b'\\' as u16).take(nslashes * 2)); + break; + } + } + } + + escaped.push(b'"' as u16); + OsString::from_wide(&escaped).into() +} + +#[cfg(test)] +mod tests { + use super::*; + extern crate test_case; + + #[test_case::test_case( + r#""""#, + r#""\"\"""# + ; "an empty string is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#""--features=\"default\"""#, + r#""\"--features=\\\"default\\\"\"""# + ; "a flag with quotes is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#"linker=gcc -L/foo -Wl,bar"#, + r#""linker=gcc -L/foo -Wl,bar""# + ; "a flag with spaces is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#"\path\to\my documents\"#, + r#""\path\to\my documents\\""# + ; "a path with spaces is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + "--aaa=bbb-ccc", + "--aaa=bbb-ccc" + ; "a flag built up entirely of allowed characters is not escaped." + )] + fn test_escape(input: &str, expected: &str) { + assert_eq!(escape(input.into()), expected); + } + + #[test_case::test_case( + r#""""#, + r#""\"\"""# + ; "an empty string is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#""--features=\"default\"""#, + r#""\"--features=\\\"default\\\"\"""# + ; "a flag with quotes is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#"linker=gcc -L/foo -Wl,bar"#, + r#""linker=gcc -L/foo -Wl,bar""# + ; "a flag with spaces is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + r#"\path\to\my documents\"#, + r#""\path\to\my documents\\""# + ; "a path with spaces is escaped by surrounding with double quotes." + )] + #[test_case::test_case( + "--aaa=bbb-ccc", + "--aaa=bbb-ccc" + ; "a flag built up entirely of allowed characters is not escaped." + )] + fn test_escape_os_str(input: &str, expected: &str) { + let binding = OsString::from(input); + let input_os_str = binding.as_os_str(); + let binding = OsString::from(expected); + let expected_os_str = binding.as_os_str(); + let observed_os_str = escape_os_str(input_os_str); + assert_eq!(observed_os_str, expected_os_str); + } + + /// FIXME: Need to fix this test case. + /// I'm not sure what we're expecting to happen here. + #[test_case::test_case( + &[0x1055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065], + &[0x1055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065] + ; "A u16 with high byte set requires escaping." + )] + fn test_escape_os_str_from_bytes(input: &[u16], expected: &[u16]) { + let binding = OsString::from_wide(input); + let input_os_str = binding.as_os_str(); + let binding = OsString::from_wide(expected); + let expected_os_str = binding.as_os_str(); + let observed_os_str = escape_os_str(input_os_str); + assert_eq!(observed_os_str, expected_os_str); + } +} \ No newline at end of file From 37d4fc876121f24c32a6d700cb5a6d4cf38ff0d6 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Mon, 13 Feb 2023 22:43:38 -0600 Subject: [PATCH 11/17] Make `escape` for windows more readable. --- src/windows.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index 2c9c5ea..eb8a245 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -11,18 +11,14 @@ use std::os::windows::ffi::{OsStrExt, OsStringExt}; /// /// [msdn]: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx pub fn escape(s: Cow) -> Cow { - let mut needs_escape = s.is_empty(); - for ch in s.chars() { - match ch { - '"' | '\t' | '\n' | ' ' => needs_escape = true, - _ => {} - } - } - if !needs_escape { + let needs_escape = s.chars().any(|c| matches!(c, '"' | '\t' | '\n' | ' ')); + if s.is_empty() || !needs_escape { return s; } + let mut es = String::with_capacity(s.len()); es.push('"'); + let mut chars = s.chars().peekable(); loop { let mut nslashes = 0; @@ -33,14 +29,17 @@ pub fn escape(s: Cow) -> Cow { match chars.next() { Some('"') => { + es.reserve(nslashes * 2 + 1); es.extend(repeat('\\').take(nslashes * 2 + 1)); es.push('"'); } Some(c) => { + es.reserve(nslashes); es.extend(repeat('\\').take(nslashes)); es.push(c); } None => { + es.reserve(nslashes * 2); es.extend(repeat('\\').take(nslashes * 2)); break; } From 60219aaffe4f23cd77a1cbe397def24ac92ba98d Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Mon, 13 Feb 2023 22:45:58 -0600 Subject: [PATCH 12/17] Reserve more bytes. --- src/windows.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index eb8a245..f0ecc3f 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -15,7 +15,7 @@ pub fn escape(s: Cow) -> Cow { if s.is_empty() || !needs_escape { return s; } - + let mut es = String::with_capacity(s.len()); es.push('"'); @@ -29,12 +29,12 @@ pub fn escape(s: Cow) -> Cow { match chars.next() { Some('"') => { - es.reserve(nslashes * 2 + 1); + es.reserve(nslashes * 2 + 2); es.extend(repeat('\\').take(nslashes * 2 + 1)); es.push('"'); } Some(c) => { - es.reserve(nslashes); + es.reserve(nslashes + 1); es.extend(repeat('\\').take(nslashes)); es.push(c); } @@ -109,12 +109,12 @@ pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { } match chars.next() { Some(c) if c == b'"' as u16 => { - escaped.reserve(nslashes * 2 + 1); + escaped.reserve(nslashes * 2 + 2); escaped.extend(repeat(b'\\' as u16).take(nslashes * 2 + 1)); escaped.push(b'"' as u16); } Some(c) => { - escaped.reserve(nslashes); + escaped.reserve(nslashes + 1); escaped.extend(repeat(b'\\' as u16).take(nslashes)); escaped.push(c); } From e94b26f1b123ba380df8afa3794745298935a217 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Mon, 13 Feb 2023 23:11:13 -0600 Subject: [PATCH 13/17] Apply some suggestions from the review. --- src/lib.rs | 4 ++-- src/windows.rs | 38 ++++++++++++++++---------------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f130fd2..5912d27 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,7 +11,7 @@ #![doc(html_root_url = "https://docs.rs/shell-escape/0.1")] #[cfg(unix)] -mod unix; +pub mod unix; #[cfg(unix)] pub use unix::{ @@ -20,7 +20,7 @@ pub use unix::{ }; #[cfg(windows)] -mod windows; +pub mod windows; #[cfg(windows)] pub use windows::{ diff --git a/src/windows.rs b/src/windows.rs index f0ecc3f..93b888e 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -16,14 +16,13 @@ pub fn escape(s: Cow) -> Cow { return s; } - let mut es = String::with_capacity(s.len()); + let mut es = String::with_capacity(s.len() + 2); es.push('"'); let mut chars = s.chars().peekable(); loop { let mut nslashes = 0; - while let Some(&'\\') = chars.peek() { - chars.next(); + while let Some(_) = chars.next_if_eq(&'\\') { nslashes += 1; } @@ -62,13 +61,10 @@ pub fn escape(s: Cow) -> Cow { /// assert!(!needs_escape(b'\\' as u16)); /// ``` pub fn needs_escape(wide_byte: u16) -> bool { - let (high, low) = ((wide_byte >> 8) as u8, (wide_byte & 0xFF) as u8); - - if high > 0 { - // High byte is set, so its not an ASCII character and definitely needs escaping. - return true; + match char::from_u32(wide_byte as u32) { + Some(c) => matches!(c, '"' | '\t' | '\n' | ' '), // only tabs, newlines, spaces, and double quotes need to be escaped + None => true } - matches!(low, b'"' | b'\t' | b'\n' | b' ') } /// Escape OsStr for the windows cmd.exe shell. @@ -86,26 +82,21 @@ pub fn needs_escape(wide_byte: u16) -> bool { /// ``` /// [msdn]: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { - let encoded: Vec = s.encode_wide().collect(); - let needs_escaping = encoded.iter().copied().any(needs_escape); + let encoded = s.encode_wide(); + let needs_escaping = encoded.clone().any(needs_escape); if s.is_empty() || !needs_escaping { return Cow::Borrowed(s); } - let mut escaped = Vec::with_capacity(encoded.len() + 2); + let mut escaped = Vec::with_capacity(s.len() + 2); escaped.push(b'"' as u16); let mut chars = encoded.into_iter().peekable(); loop { let mut nslashes = 0; - while let Some(&c) = chars.peek() { - if c == (b'\\' as u16) { - chars.next(); - nslashes += 1; - } else { - break; - } + while let Some(_) = chars.next_if_eq(&(b'\\' as u16)) { + nslashes += 1; } match chars.next() { Some(c) if c == b'"' as u16 => { @@ -198,12 +189,15 @@ mod tests { assert_eq!(observed_os_str, expected_os_str); } - /// FIXME: Need to fix this test case. - /// I'm not sure what we're expecting to happen here. #[test_case::test_case( &[0x1055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065], &[0x1055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065] - ; "A u16 with high byte set requires escaping." + ; "u16 (as u32) that are valid chars are not escaped unless they are a double quote, space, backslash, newline, or a tab." + )] + #[test_case::test_case( + &[0xD801, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065], + &[b'"' as u16, 0xD801, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065, b'"' as u16] + ; "a 16-bit number that is not a valid char when seen as a u32 is escaped by surrounding with double quotes." )] fn test_escape_os_str_from_bytes(input: &[u16], expected: &[u16]) { let binding = OsString::from_wide(input); From 13def301d8eef63fd8da24cccdeacfcdec62f4d3 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Mon, 13 Feb 2023 23:11:57 -0600 Subject: [PATCH 14/17] Apply clippy fixes. --- src/windows.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index 93b888e..7291bb3 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -22,7 +22,7 @@ pub fn escape(s: Cow) -> Cow { let mut chars = s.chars().peekable(); loop { let mut nslashes = 0; - while let Some(_) = chars.next_if_eq(&'\\') { + while chars.next_if_eq(&'\\').is_some() { nslashes += 1; } @@ -95,7 +95,7 @@ pub fn escape_os_str(s: &OsStr) -> Cow<'_, OsStr> { let mut chars = encoded.into_iter().peekable(); loop { let mut nslashes = 0; - while let Some(_) = chars.next_if_eq(&(b'\\' as u16)) { + while chars.next_if_eq(&(b'\\' as u16)).is_some() { nslashes += 1; } match chars.next() { From c0d2a8b31ae931855b7bdf47376b8411df92a8bc Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Mon, 13 Feb 2023 23:12:36 -0600 Subject: [PATCH 15/17] Rustfmt fixes. --- src/lib.rs | 10 ++-------- src/unix.rs | 4 ++-- src/windows.rs | 6 +++--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5912d27..39c3d10 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,16 +14,10 @@ pub mod unix; #[cfg(unix)] -pub use unix::{ - escape, - escape_os_str -}; +pub use unix::{escape, escape_os_str}; #[cfg(windows)] pub mod windows; #[cfg(windows)] -pub use windows::{ - escape, - escape_os_str -}; +pub use windows::{escape, escape_os_str}; diff --git a/src/unix.rs b/src/unix.rs index e95f8df..d55404f 100644 --- a/src/unix.rs +++ b/src/unix.rs @@ -1,5 +1,5 @@ //! Unix-specific escaping. -//! +//! use std::{ borrow::Cow, ffi::{OsStr, OsString}, @@ -165,4 +165,4 @@ mod tests { let expected_os_str = OsStr::from_bytes(expected); assert_eq!(observed_os_str, expected_os_str); } -} \ No newline at end of file +} diff --git a/src/windows.rs b/src/windows.rs index 7291bb3..4844afe 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -18,7 +18,7 @@ pub fn escape(s: Cow) -> Cow { let mut es = String::with_capacity(s.len() + 2); es.push('"'); - + let mut chars = s.chars().peekable(); loop { let mut nslashes = 0; @@ -63,7 +63,7 @@ pub fn escape(s: Cow) -> Cow { pub fn needs_escape(wide_byte: u16) -> bool { match char::from_u32(wide_byte as u32) { Some(c) => matches!(c, '"' | '\t' | '\n' | ' '), // only tabs, newlines, spaces, and double quotes need to be escaped - None => true + None => true, } } @@ -207,4 +207,4 @@ mod tests { let observed_os_str = escape_os_str(input_os_str); assert_eq!(observed_os_str, expected_os_str); } -} \ No newline at end of file +} From b0bc985f424edf69b32640b0508b72f9fee79ec8 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Mon, 13 Feb 2023 23:31:18 -0600 Subject: [PATCH 16/17] Fix doctest import. --- src/windows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows.rs b/src/windows.rs index 4844afe..845b64b 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -73,7 +73,7 @@ pub fn needs_escape(wide_byte: u16) -> bool { /// /// Example: /// ``` -/// use shell_escape::windows::escape_os_str; +/// use shell_escape::escape_os_str; /// use std::ffi::OsStr; /// /// let s = OsStr::new("foo bar"); From 8b281f2e5ed864d80ddf2898aa108e677bf053c0 Mon Sep 17 00:00:00 2001 From: Aalekh Patel Date: Wed, 1 Mar 2023 09:19:06 -0600 Subject: [PATCH 17/17] Make the implementation of escape uniform across both platforms. --- src/unix.rs | 7 ++++++- src/windows.rs | 12 ++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/unix.rs b/src/unix.rs index d55404f..a5c8107 100644 --- a/src/unix.rs +++ b/src/unix.rs @@ -12,7 +12,12 @@ fn non_whitelisted(ch: char) -> bool { /// Escape characters that may have special meaning in a shell, including spaces. pub fn escape(s: Cow) -> Cow { - if !s.is_empty() && !s.contains(non_whitelisted) { + if s.is_empty() { + return "''".to_string().into(); + } + + let needs_escape = s.chars().any(non_whitelisted); + if !needs_escape { return s; } diff --git a/src/windows.rs b/src/windows.rs index 845b64b..879eaa1 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -5,14 +5,22 @@ use std::ffi::{OsStr, OsString}; use std::iter::repeat; use std::os::windows::ffi::{OsStrExt, OsStringExt}; +fn non_whitelisted(c: char) -> bool { + !matches!(c, '"' | '\t' | '\n' | ' ') +} + /// Escape for the windows cmd.exe shell. /// /// See [here][msdn] for more information. /// /// [msdn]: http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx pub fn escape(s: Cow) -> Cow { - let needs_escape = s.chars().any(|c| matches!(c, '"' | '\t' | '\n' | ' ')); - if s.is_empty() || !needs_escape { + if s.is_empty() { + return "\"\"".to_string().into(); + } + + let needs_escape = s.chars().any(non_whitelisted); + if !needs_escape { return s; }