Skip to content

Commit b45def2

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 b45def2

File tree

4 files changed

+87
-12
lines changed

4 files changed

+87
-12
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/process/tests.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ fn debug_print() {
552552

553553
let mut command = Command::new("some-boring-name");
554554

555-
assert_eq!(format!("{command:?}"), format!(r#""some-boring-name""#));
555+
assert_eq!(format!("{command:?}"), format!("some-boring-name"));
556556

557557
assert_eq!(
558558
format!("{command:#?}"),
@@ -568,7 +568,7 @@ fn debug_print() {
568568

569569
command.args(&["1", "2", "3"]);
570570

571-
assert_eq!(format!("{command:?}"), format!(r#""some-boring-name" "1" "2" "3""#));
571+
assert_eq!(format!("{command:?}"), format!("some-boring-name 1 2 3"));
572572

573573
assert_eq!(
574574
format!("{command:#?}"),
@@ -589,7 +589,7 @@ fn debug_print() {
589589

590590
assert_eq!(
591591
format!("{command:?}"),
592-
format!(r#"["some-boring-name"] "exciting-name" "1" "2" "3""#)
592+
format!("[some-boring-name] exciting-name 1 2 3")
593593
);
594594

595595
assert_eq!(
@@ -611,7 +611,7 @@ fn debug_print() {
611611
command_with_env_and_cwd.current_dir("/some/path").env("FOO", "bar");
612612
assert_eq!(
613613
format!("{command_with_env_and_cwd:?}"),
614-
r#"cd "/some/path" && FOO="bar" "boring-name""#
614+
"cd /some/path && FOO=bar boring-name"
615615
);
616616
assert_eq!(
617617
format!("{command_with_env_and_cwd:#?}"),
@@ -638,7 +638,7 @@ fn debug_print() {
638638

639639
let mut command_with_removed_env = Command::new("boring-name");
640640
command_with_removed_env.env_remove("FOO").env_remove("BAR");
641-
assert_eq!(format!("{command_with_removed_env:?}"), r#"env -u BAR -u FOO "boring-name""#);
641+
assert_eq!(format!("{command_with_removed_env:?}"), "env -u BAR -u FOO boring-name");
642642
assert_eq!(
643643
format!("{command_with_removed_env:#?}"),
644644
format!(
@@ -660,7 +660,7 @@ fn debug_print() {
660660

661661
let mut command_with_cleared_env = Command::new("boring-name");
662662
command_with_cleared_env.env_clear().env("BAR", "val").env_remove("FOO");
663-
assert_eq!(format!("{command_with_cleared_env:?}"), r#"env -i BAR="val" "boring-name""#);
663+
assert_eq!(format!("{command_with_cleared_env:?}"), "env -i BAR=val boring-name");
664664
assert_eq!(
665665
format!("{command_with_cleared_env:#?}"),
666666
format!(

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

+47-6
Original file line numberDiff line numberDiff line change
@@ -580,8 +580,43 @@ 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], is_arg: bool) -> 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+
let can_print_safely = |c| {
589+
// See: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02
590+
match c {
591+
// The documentation linked above says that `=`:
592+
// > may need to be quoted under certain circumstances
593+
//
594+
// Because they may be parsed as a variable assignment.
595+
// But in argument position to a command, they
596+
// shouldn't need escaping.
597+
b'=' if is_arg => true,
598+
b'|' | b'&' | b';' | b'<' | b'>' | b'(' | b')' | b'$' | b'`' | b'\\'
599+
| b'"' | b'\'' | b' ' | b'\t' | b'\n' => false,
600+
b'*' | b'?' | b'[' | b'#' | b'~' | b'=' | b'%' => false,
601+
// Assume all other printable characthers may be output.
602+
0x20..0x7e => true,
603+
// Unprintable or not ASCII.
604+
_ => false,
605+
}
606+
};
607+
fmt::from_fn(move |f| {
608+
if !bytes.is_empty() && bytes.iter().all(can_print_safely) {
609+
// Unwrap is fine, we've checked above that the bytes only contains ascii.
610+
let ascii = crate::str::from_utf8(bytes).unwrap();
611+
write!(f, "{ascii}")
612+
} else {
613+
write!(f, "\"{}\"", bytes.escape_ascii())
614+
}
615+
})
616+
}
617+
583618
if let Some(ref cwd) = self.cwd {
584-
write!(f, "cd {cwd:?} && ")?;
619+
write!(f, "cd {} && ", relaxed_escaping(cwd.to_bytes(), true))?;
585620
}
586621
if self.env.does_clear() {
587622
write!(f, "env -i ")?;
@@ -595,23 +630,29 @@ impl fmt::Debug for Command {
595630
write!(f, "env ")?;
596631
any_removed = true;
597632
}
598-
write!(f, "-u {} ", key.to_string_lossy())?;
633+
write!(f, "-u {} ", relaxed_escaping(key.as_encoded_bytes(), false))?;
599634
}
600635
}
601636
}
602637
// Altered env vars can just be added in front of the program.
603638
for (key, value_opt) in self.get_envs() {
604639
if let Some(value) = value_opt {
605-
write!(f, "{}={value:?} ", key.to_string_lossy())?;
640+
write!(
641+
f,
642+
"{}={} ",
643+
relaxed_escaping(key.as_encoded_bytes()),
644+
relaxed_escaping(value.as_encoded_bytes())
645+
)?;
606646
}
607647
}
608648
if self.program != self.args[0] {
609-
write!(f, "[{:?}] ", self.program)?;
649+
write!(f, "[{}] ", relaxed_escaping(self.program.to_bytes(), false))?;
610650
}
611-
write!(f, "{:?}", self.args[0])?;
651+
652+
write!(f, "{}", relaxed_escaping(self.args[0].to_bytes(), false))?;
612653

613654
for arg in &self.args[1..] {
614-
write!(f, " {:?}", arg)?;
655+
write!(f, " {}", relaxed_escaping(arg.to_bytes(), true))?;
615656
}
616657
Ok(())
617658
}

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("", r#""""#);
204+
check("é", r#""\xc3\xa9""#);
205+
check("a'", r#""a\'""#);
206+
check("a\"", r#""a\"""#);
207+
check(" ", r#"" ""#);
208+
check(
209+
"/Users/The user's name/.cargo/bin/cargo",
210+
r#""/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:?}"), r#"ld my/file.o -macos_version_min 13.0 "-arch arm64""#);
225+
}

0 commit comments

Comments
 (0)