fix(shell): robust cmd detection + stronger test assertion (review #1744)
Address gemini-code-assist review on PR #1744 (two MEDIUM): - cmd detection used program.eq_ignore_ascii_case("cmd"), which fails for a full path (C:\Windows\System32\cmd.exe) or a .exe suffix, so the raw_arg quoting fix would not apply. Use Path::file_stem() instead (fully-qualified std::path::Path -> no unused import off-Windows). - Strengthen the Windows block of issue_1691_quoted_commit_message_round_trips to assert argv content equals spec.args, not just arg count, so the raw_arg payload (quotes preserved, no extra escaping) is actually verified. sandbox/mod.rs already asserts content -- left untouched. Windows paths are cfg-gated (compile-checked on macOS, executed on Windows CI). macOS build + clippy clean. Refs #1691.
This commit is contained in:
committed by
Hunter Bown
parent
b3223cd48d
commit
0ce41505bc
@@ -192,8 +192,15 @@ fn push_shell_args(cmd: &mut Command, program: &str, args: &[String]) {
|
||||
use std::os::windows::process::CommandExt;
|
||||
// The `cmd /C <payload>` shape is the only place std's per-arg escaping
|
||||
// corrupts a quoted command. Pass `/C` and the payload raw so the quotes
|
||||
// survive; any other program keeps normal (correct) escaping.
|
||||
if program.eq_ignore_ascii_case("cmd")
|
||||
// survive; any other program keeps normal (correct) escaping. Match `cmd`
|
||||
// by file stem so a full path (`C:\Windows\System32\cmd.exe`) or `.exe`
|
||||
// suffix still triggers the raw-arg path.
|
||||
let is_cmd = std::path::Path::new(program)
|
||||
.file_stem()
|
||||
.and_then(|s| s.to_str())
|
||||
.map(|s| s.eq_ignore_ascii_case("cmd"))
|
||||
.unwrap_or(false);
|
||||
if is_cmd
|
||||
&& args.len() == 2
|
||||
&& args[0].eq_ignore_ascii_case("/C")
|
||||
{
|
||||
|
||||
@@ -852,6 +852,10 @@ fn issue_1691_quoted_commit_message_round_trips() {
|
||||
);
|
||||
let mut built = Command::new(&spec.program);
|
||||
push_shell_args(&mut built, &spec.program, &spec.args);
|
||||
assert_eq!(built.get_args().count(), 2);
|
||||
let got: Vec<String> = built
|
||||
.get_args()
|
||||
.map(|a| a.to_string_lossy().into_owned())
|
||||
.collect();
|
||||
assert_eq!(got, spec.args);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user