Skip to content

Commit db8a0e7

Browse files
committed
unix: Relax escaping in Debug impl on Command
The Debug output of Command is very useful for showing to the user the command that was executed when something went wrong. This is done for example by rustc when invoking an external tool like the linker fails. It is also overly verbose, since everything is quoted, which makes it harder to read. Instead, we now first check if we're reasonably sure that an argument is simple enough that using it in the shell wouldn't need quoting, and then output it without quotes if possible. Before and example output could look like this: PATH="/a:/b" "cc" "foo.o" "-target" "arm64-apple-darwin11.0" Now it looks like this: PATH="/a:/b" cc foo.o -target arm64-apple-darwin11.0
1 parent 705cfe0 commit db8a0e7

File tree

3 files changed

+67
-6
lines changed

3 files changed

+67
-6
lines changed

library/std/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@
291291
#![feature(cfi_encoding)]
292292
#![feature(concat_idents)]
293293
#![feature(const_float_methods)]
294+
#![feature(debug_closure_helpers)]
294295
#![feature(decl_macro)]
295296
#![feature(deprecated_suggestion)]
296297
#![feature(doc_cfg)]

library/std/src/sys/pal/unix/process/process_common.rs

+33-6
Original file line numberDiff line numberDiff line change
@@ -580,8 +580,29 @@ impl fmt::Debug for Command {
580580

581581
debug_command.finish()
582582
} else {
583+
/// Only escape arguments when necessary, otherwise output it
584+
/// unescaped to make the output easier to read.
585+
fn relaxed_escaping(bytes: &[u8]) -> impl fmt::Display + use<'_> {
586+
// Don't escape if all the characters are likely
587+
// to not be a problem when copied to the shell.
588+
fmt::from_fn(move |f| {
589+
if !bytes.is_empty()
590+
&& bytes.iter().all(|c| {
591+
c.is_ascii_alphanumeric()
592+
|| matches!(c, b'_' | b'/' | b'-' | b'=' | b'.' | b',')
593+
})
594+
{
595+
// Unwrap is fine, we've checked above that the bytes only contains ascii.
596+
let ascii = crate::str::from_utf8(bytes).unwrap();
597+
write!(f, "{ascii}")
598+
} else {
599+
write!(f, "\"{}\"", bytes.escape_ascii())
600+
}
601+
})
602+
}
603+
583604
if let Some(ref cwd) = self.cwd {
584-
write!(f, "cd {cwd:?} && ")?;
605+
write!(f, "cd {} && ", relaxed_escaping(cwd.to_bytes()))?;
585606
}
586607
if self.env.does_clear() {
587608
write!(f, "env -i ")?;
@@ -595,23 +616,29 @@ impl fmt::Debug for Command {
595616
write!(f, "env ")?;
596617
any_removed = true;
597618
}
598-
write!(f, "-u {} ", key.to_string_lossy())?;
619+
write!(f, "-u {} ", relaxed_escaping(key.as_encoded_bytes()))?;
599620
}
600621
}
601622
}
602623
// Altered env vars can just be added in front of the program.
603624
for (key, value_opt) in self.get_envs() {
604625
if let Some(value) = value_opt {
605-
write!(f, "{}={value:?} ", key.to_string_lossy())?;
626+
write!(
627+
f,
628+
"{}={} ",
629+
relaxed_escaping(key.as_encoded_bytes()),
630+
relaxed_escaping(value.as_encoded_bytes())
631+
)?;
606632
}
607633
}
608634
if self.program != self.args[0] {
609-
write!(f, "[{:?}] ", self.program)?;
635+
write!(f, "[{}] ", relaxed_escaping(self.program.to_bytes()))?;
610636
}
611-
write!(f, "{:?}", self.args[0])?;
637+
638+
write!(f, "{}", relaxed_escaping(self.args[0].to_bytes()))?;
612639

613640
for arg in &self.args[1..] {
614-
write!(f, " {:?}", arg)?;
641+
write!(f, " {}", relaxed_escaping(arg.to_bytes()))?;
615642
}
616643
Ok(())
617644
}

library/std/src/sys/pal/unix/process/process_common/tests.rs

+33
Original file line numberDiff line numberDiff line change
@@ -190,3 +190,36 @@ fn unix_exit_statuses() {
190190
}
191191
}
192192
}
193+
194+
#[test]
195+
fn command_debug_escaping() {
196+
#[track_caller]
197+
fn check(arg: impl AsRef<OsStr>, expected: &str) {
198+
let cmd = Command::new(arg.as_ref());
199+
assert_eq!(format!("{cmd:?}"), expected);
200+
}
201+
202+
// Escaped.
203+
check("", "\"\"");
204+
check("é", "\"\\xc3\\xa9\"");
205+
check("a'", "\"a\\'\"");
206+
check("a\"", "\"a\\\"\"");
207+
check(" ", "\" \"");
208+
check(
209+
"/Users/The user's name/.cargo/bin/cargo",
210+
"\"/Users/The user\\'s name/.cargo/bin/cargo\"",
211+
);
212+
213+
// Simple enough that we don't escape them.
214+
check("a", "a");
215+
check("/my/simple/path", "/my/simple/path");
216+
check("abc-defg_1234", "abc-defg_1234");
217+
218+
// Real-world use-case, immediately clear that two of the arguments are passed as one.
219+
let mut cmd = crate::process::Command::new("ld");
220+
cmd.arg("my/file.o");
221+
cmd.arg("-macos_version_min");
222+
cmd.arg("13.0");
223+
cmd.arg("-arch arm64");
224+
assert_eq!(format!("{cmd:?}"), "ld my/file.o -macos_version_min 13.0 \"-arch arm64\"");
225+
}

0 commit comments

Comments
 (0)