diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index ee6fceb024fd7..937a1973c1f49 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -285,6 +285,7 @@ #![feature(cfg_target_thread_local)] #![feature(cfi_encoding)] #![feature(concat_idents)] +#![feature(debug_closure_helpers)] #![feature(decl_macro)] #![feature(deprecated_suggestion)] #![feature(doc_cfg)] diff --git a/library/std/src/process/tests.rs b/library/std/src/process/tests.rs index fb0b495961c36..308384920d92f 100644 --- a/library/std/src/process/tests.rs +++ b/library/std/src/process/tests.rs @@ -552,7 +552,7 @@ fn debug_print() { let mut command = Command::new("some-boring-name"); - assert_eq!(format!("{command:?}"), format!(r#""some-boring-name""#)); + assert_eq!(format!("{command:?}"), format!("some-boring-name")); assert_eq!( format!("{command:#?}"), @@ -568,7 +568,7 @@ fn debug_print() { command.args(&["1", "2", "3"]); - assert_eq!(format!("{command:?}"), format!(r#""some-boring-name" "1" "2" "3""#)); + assert_eq!(format!("{command:?}"), format!("some-boring-name 1 2 3")); assert_eq!( format!("{command:#?}"), @@ -587,10 +587,7 @@ fn debug_print() { crate::os::unix::process::CommandExt::arg0(&mut command, "exciting-name"); - assert_eq!( - format!("{command:?}"), - format!(r#"["some-boring-name"] "exciting-name" "1" "2" "3""#) - ); + assert_eq!(format!("{command:?}"), format!("[some-boring-name] exciting-name 1 2 3")); assert_eq!( format!("{command:#?}"), @@ -609,10 +606,7 @@ fn debug_print() { let mut command_with_env_and_cwd = Command::new("boring-name"); command_with_env_and_cwd.current_dir("/some/path").env("FOO", "bar"); - assert_eq!( - format!("{command_with_env_and_cwd:?}"), - r#"cd "/some/path" && FOO="bar" "boring-name""# - ); + assert_eq!(format!("{command_with_env_and_cwd:?}"), "cd /some/path && FOO=bar boring-name"); assert_eq!( format!("{command_with_env_and_cwd:#?}"), format!( @@ -638,7 +632,7 @@ fn debug_print() { let mut command_with_removed_env = Command::new("boring-name"); command_with_removed_env.env_remove("FOO").env_remove("BAR"); - assert_eq!(format!("{command_with_removed_env:?}"), r#"env -u BAR -u FOO "boring-name""#); + assert_eq!(format!("{command_with_removed_env:?}"), "env -u BAR -u FOO boring-name"); assert_eq!( format!("{command_with_removed_env:#?}"), format!( @@ -660,7 +654,7 @@ fn debug_print() { let mut command_with_cleared_env = Command::new("boring-name"); command_with_cleared_env.env_clear().env("BAR", "val").env_remove("FOO"); - assert_eq!(format!("{command_with_cleared_env:?}"), r#"env -i BAR="val" "boring-name""#); + assert_eq!(format!("{command_with_cleared_env:?}"), "env -i BAR=val boring-name"); assert_eq!( format!("{command_with_cleared_env:#?}"), format!( diff --git a/library/std/src/sys/pal/unix/process/process_common.rs b/library/std/src/sys/pal/unix/process/process_common.rs index 13290fed762ae..b8619b896992d 100644 --- a/library/std/src/sys/pal/unix/process/process_common.rs +++ b/library/std/src/sys/pal/unix/process/process_common.rs @@ -580,8 +580,56 @@ impl fmt::Debug for Command { debug_command.finish() } else { + /// Only escape arguments when necessary, otherwise output it + /// unescaped to make the output easier to read. + /// + /// NOTE: This is best effort only! + fn relaxed_escaping(bytes: &[u8], is_arg: bool) -> impl fmt::Display + use<'_> { + // Don't escape if all the characters are likely + // to not be a problem when copied to the shell. + let can_print_safely = move |&c| { + // See: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02 + match c { + // The documentation linked above says that `=`: + // > may need to be quoted under certain circumstances + // + // Because they may be parsed as a variable assignment. + // But in argument position to a command, they + // shouldn't need escaping. + b'=' if is_arg => true, + // As per documentation linked above: + // > The application shall quote the following characters + b'|' | b'&' | b';' | b'<' | b'>' | b'(' | b')' | b'$' | b'`' | b'\\' + | b'"' | b'\'' | b' ' | b'\t' | b'\n' => false, + // As per documentation linked above: + // > may need to be quoted under certain circumstances + b'*' | b'?' | b'[' | b'#' | b'~' | b'=' | b'%' => false, + // It'd be weird to quote `[` and not quote `]`. + b']' => false, + // ! does history expansion in Bash, require quoting for that as well. + // + // NOTE: This doesn't currently work properly, we'd need to backslash-escape + // `!` as well (which `escape_ascii` doesn't do). + b'!' => false, + // Assume all other printable characters may be output. + 0x20..0x7e => true, + // Unprintable or not ASCII. + _ => false, + } + }; + fmt::from_fn(move |f| { + if !bytes.is_empty() && bytes.iter().all(can_print_safely) { + // Unwrap is fine, we've checked above that the bytes only contains ascii. + let ascii = crate::str::from_utf8(bytes).unwrap(); + write!(f, "{ascii}") + } else { + write!(f, "\"{}\"", bytes.escape_ascii()) + } + }) + } + if let Some(ref cwd) = self.cwd { - write!(f, "cd {cwd:?} && ")?; + write!(f, "cd {} && ", relaxed_escaping(cwd.to_bytes(), true))?; } if self.env.does_clear() { write!(f, "env -i ")?; @@ -595,23 +643,29 @@ impl fmt::Debug for Command { write!(f, "env ")?; any_removed = true; } - write!(f, "-u {} ", key.to_string_lossy())?; + write!(f, "-u {} ", relaxed_escaping(key.as_encoded_bytes(), false))?; } } } // Altered env vars can just be added in front of the program. for (key, value_opt) in self.get_envs() { if let Some(value) = value_opt { - write!(f, "{}={value:?} ", key.to_string_lossy())?; + write!( + f, + "{}={} ", + relaxed_escaping(key.as_encoded_bytes(), false), + relaxed_escaping(value.as_encoded_bytes(), false) + )?; } } if self.program != self.args[0] { - write!(f, "[{:?}] ", self.program)?; + write!(f, "[{}] ", relaxed_escaping(self.program.to_bytes(), false))?; } - write!(f, "{:?}", self.args[0])?; + + write!(f, "{}", relaxed_escaping(self.args[0].to_bytes(), false))?; for arg in &self.args[1..] { - write!(f, " {:?}", arg)?; + write!(f, " {}", relaxed_escaping(arg.to_bytes(), true))?; } Ok(()) } diff --git a/library/std/src/sys/pal/unix/process/process_common/tests.rs b/library/std/src/sys/pal/unix/process/process_common/tests.rs index e5c8dd6e341e1..86b542bac931d 100644 --- a/library/std/src/sys/pal/unix/process/process_common/tests.rs +++ b/library/std/src/sys/pal/unix/process/process_common/tests.rs @@ -190,3 +190,38 @@ fn unix_exit_statuses() { } } } + +#[test] +fn command_debug_escaping() { + #[track_caller] + fn check(arg: impl AsRef, expected: &str) { + let cmd = Command::new(arg.as_ref()); + assert_eq!(format!("{cmd:?}"), expected); + } + + // Escaped. + check("", r#""""#); + check("é", r#""\xc3\xa9""#); + check("a'", r#""a\'""#); + check("a\"", r#""a\"""#); + check("'\"", r#""\'\"""#); + check(" ", r#"" ""#); + check( + "/Users/The user's name/.cargo/bin/cargo", + r#""/Users/The user\'s name/.cargo/bin/cargo""#, + ); + check("!a", r#""!a""#); + + // Simple enough that we don't escape them. + check("a", "a"); + check("/my/simple/path", "/my/simple/path"); + check("abc-defg_1234", "abc-defg_1234"); + + // Real-world use-case, immediately clear that two of the arguments are passed as one. + let mut cmd = crate::process::Command::new("ld"); + cmd.arg("my/file.o"); + cmd.arg("-macos_version_min"); + cmd.arg("13.0"); + cmd.arg("-arch arm64"); + assert_eq!(format!("{cmd:?}"), r#"ld my/file.o -macos_version_min 13.0 "-arch arm64""#); +} diff --git a/tests/run-make/link-args-order/rmake.rs b/tests/run-make/link-args-order/rmake.rs index b7ef8333267f2..8075dd23e6068 100644 --- a/tests/run-make/link-args-order/rmake.rs +++ b/tests/run-make/link-args-order/rmake.rs @@ -13,17 +13,19 @@ fn main() { .linker_flavor(linker) .link_arg("a") .link_args("b c") - .link_args("d e") - .link_arg("f") + .link_arg("d e") + .link_args("f g") + .link_arg("h") .run_fail() - .assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#); + .assert_stderr_contains(r#"a b c "d e" f g h"#); rustc() .input("empty.rs") .linker_flavor(linker) .arg("-Zpre-link-arg=a") .arg("-Zpre-link-args=b c") - .arg("-Zpre-link-args=d e") - .arg("-Zpre-link-arg=f") + .arg("-Zpre-link-arg=d e") + .arg("-Zpre-link-args=f g") + .arg("-Zpre-link-arg=h") .run_fail() - .assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#); + .assert_stderr_contains(r#"a b c "d e" f g h"#); } diff --git a/tests/run-make/link-dedup/rmake.rs b/tests/run-make/link-dedup/rmake.rs index 6075f31095424..0075057b340ca 100644 --- a/tests/run-make/link-dedup/rmake.rs +++ b/tests/run-make/link-dedup/rmake.rs @@ -31,9 +31,9 @@ fn needle_from_libs(libs: &[&str]) -> String { let mut needle = String::new(); for lib in libs { if is_msvc() { - let _ = needle.write_fmt(format_args!(r#""{lib}.lib" "#)); + let _ = needle.write_fmt(format_args!("{lib}.lib ")); } else { - let _ = needle.write_fmt(format_args!(r#""-l{lib}" "#)); + let _ = needle.write_fmt(format_args!("-l{lib} ")); } } needle.pop(); // remove trailing space diff --git a/tests/run-make/pass-linker-flags-flavor/rmake.rs b/tests/run-make/pass-linker-flags-flavor/rmake.rs index e36d68cc88432..6b2ee63b28cb1 100644 --- a/tests/run-make/pass-linker-flags-flavor/rmake.rs +++ b/tests/run-make/pass-linker-flags-flavor/rmake.rs @@ -72,8 +72,8 @@ fn main() { .stdout_utf8(); let no_verbatim = regex::Regex::new("l1.*-Wl,a1.*l2.*-Wl,a2.*d1.*-Wl,a3").unwrap(); - let one_verbatim = regex::Regex::new(r#"l1.*"a1".*l2.*-Wl,a2.*d1.*-Wl,a3"#).unwrap(); - let ld = regex::Regex::new(r#"l1.*"a1".*l2.*"a2".*d1.*"a3""#).unwrap(); + let one_verbatim = regex::Regex::new("l1.*a1.*l2.*-Wl,a2.*d1.*-Wl,a3").unwrap(); + let ld = regex::Regex::new("l1.*a1.*l2.*a2.*d1.*a3").unwrap(); assert!(no_verbatim.is_match(&out_gnu)); assert!(no_verbatim.is_match(&out_att_gnu));