diff --git a/src/main.rs b/src/main.rs index c8d9a2f0a6..a9d251e677 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1247,13 +1247,23 @@ enum GoCommands { } fn run_fallback(parse_error: clap::Error) -> Result { - let args: Vec = std::env::args().skip(1).collect(); + // Non-UTF-8 argv is frequently *why* parsing failed (clap rejects it as + // InvalidUtf8), so this path must never assume valid UTF-8: std::env::args() + // would panic on exactly the input that routed us here. + let args_os: Vec = std::env::args_os().skip(1).collect(); // No args → show Clap's error (user ran just "rtk" with bad syntax) - if args.is_empty() { + if args_os.is_empty() { parse_error.exit(); } + // Lossy copies drive matching, display and tracking; execution below forwards + // the original `args_os` bytes so the wrapped tool receives what the user typed. + let args: Vec = args_os + .iter() + .map(|a| a.to_string_lossy().into_owned()) + .collect(); + // RTK meta-commands should never fall back to raw execution. // e.g. `rtk gain --badtypo` should show Clap's error, not try to run `gain` from $PATH. if core::constants::RTK_META_COMMANDS.contains(&args[0].as_str()) { @@ -1289,14 +1299,14 @@ fn run_fallback(parse_error: clap::Error) -> Result { let result = if filter.filter_stderr { // Merge stderr into stdout so the filter can strip banners emitted by tools like liquibase core::utils::resolved_command(&args[0]) - .args(&args[1..]) + .args(&args_os[1..]) .stdin(std::process::Stdio::inherit()) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) // captured for merging .output() } else { core::utils::resolved_command(&args[0]) - .args(&args[1..]) + .args(&args_os[1..]) .stdin(std::process::Stdio::inherit()) .stdout(std::process::Stdio::piped()) // capture .stderr(std::process::Stdio::inherit()) // stderr always direct @@ -1365,7 +1375,7 @@ fn run_fallback(parse_error: clap::Error) -> Result { } else { // No TOML match: original passthrough behaviour (Stdio::inherit, streaming) let status = core::utils::resolved_command(&args[0]) - .args(&args[1..]) + .args(&args_os[1..]) .stdin(std::process::Stdio::inherit()) .stdout(std::process::Stdio::inherit()) .stderr(std::process::Stdio::inherit()) diff --git a/tests/non_utf8_argv_test.rs b/tests/non_utf8_argv_test.rs new file mode 100644 index 0000000000..a41ad6b97e --- /dev/null +++ b/tests/non_utf8_argv_test.rs @@ -0,0 +1,98 @@ +#![cfg(unix)] +//! Non-UTF-8 bytes in argv must never abort rtk (#3025). Clap rejects such args as +//! InvalidUtf8, which routes them into the raw-execution fallback — so that fallback +//! must read argv without assuming UTF-8, and must forward the original bytes to the +//! wrapped tool rather than a lossy reinterpretation of them. + +use std::ffi::OsString; +use std::os::unix::ffi::OsStringExt; +use std::process::Command; + +/// Leading bytes of a double-encoded UTF-8 sequence: invalid UTF-8 on its own, and +/// the kind of fragment you grep for when hunting mojibake. From the issue repro. +const INVALID_PATTERN: &[u8] = b"\xc3\xa2\xe2\x80"; + +/// A line the pattern above matches, written as raw bytes. +const MATCHING_LINE: &[u8] = b"\xc3\xa2\xe2\x80\xa0\n"; + +fn os(bytes: &[u8]) -> OsString { + OsString::from_vec(bytes.to_vec()) +} + +fn rtk(args: &[OsString]) -> std::process::Output { + Command::new(env!("CARGO_BIN_EXE_rtk")) + .args(args) + .output() + .expect("rtk") +} + +#[test] +fn non_utf8_pattern_does_not_abort() { + let d = tempfile::tempdir().expect("create tempdir"); + let f = d.path().join("mojibake.txt"); + std::fs::write(&f, MATCHING_LINE).expect("write mojibake fixture"); + + let out = rtk(&[ + os(b"grep"), + os(b"-c"), + os(INVALID_PATTERN), + f.clone().into_os_string(), + ]); + + // Before the fix this aborted: SIGABRT (code() == None) under the release + // profile's panic="abort", exit 101 under a debug build. + assert_eq!( + out.status.code(), + Some(0), + "expected a clean exit, got {:?} — stderr: {}", + out.status, + String::from_utf8_lossy(&out.stderr) + ); + assert!( + !String::from_utf8_lossy(&out.stderr).contains("panicked"), + "must not panic: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +#[test] +fn non_utf8_pattern_forwards_original_bytes() { + let d = tempfile::tempdir().expect("create tempdir"); + let f = d.path().join("mojibake.txt"); + std::fs::write(&f, MATCHING_LINE).expect("write mojibake fixture"); + + let rtk_out = rtk(&[ + os(b"grep"), + os(b"-c"), + os(INVALID_PATTERN), + f.clone().into_os_string(), + ]); + + // The bytes must reach the engine intact: a lossy conversion would search for + // replacement characters instead and silently report zero matches. + let native = Command::new("grep") + .args([os(b"-c"), os(INVALID_PATTERN), f.into_os_string()]) + .env("LC_ALL", "C") + .output() + .expect("grep"); + + assert_eq!( + String::from_utf8_lossy(&rtk_out.stdout).trim(), + String::from_utf8_lossy(&native.stdout).trim(), + "rtk must report the same match count as native grep" + ); +} + +#[test] +fn non_utf8_arg_to_missing_command_exits_cleanly() { + // The fallback's spawn-failure path also walks argv; it must degrade to the + // usual 127 rather than aborting. + let out = rtk(&[os(b"rtk_no_such_command_xyz"), os(INVALID_PATTERN)]); + + assert_eq!(out.status.code(), Some(127), "expected command-not-found"); + assert!( + !String::from_utf8_lossy(&out.stderr).contains("panicked"), + "must not panic: {}", + String::from_utf8_lossy(&out.stderr) + ); +}