improve patch

Author: curious-rabbit

src/dir_entry.rs

@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::cell::OnceCell;
 use std::ffi::OsString;
 use std::fs::{FileType, Metadata};
@@ -54,18 +55,26 @@ impl DirEntry {
     }
 
     /// Returns the path as it should be presented to the user.
-    pub fn stripped_path(&self, config: &Config) -> &Path {
+    /// When stripping `./` would leave the path starting with `-`, keep the `./` so
+    /// downstream tools don't interpret the filename as an option.
+    pub fn stripped_path(&self, config: &Config) -> Cow<'_, Path> {
+        let path = self.path();
         if config.strip_cwd_prefix {
-            strip_current_dir(self.path())
+            let stripped = strip_current_dir(path);
+            if starts_with_dash(stripped) {
+                Cow::Owned(Path::new(".").join(stripped))
+            } else {
+                Cow::Borrowed(stripped)
+            }
         } else {
-            self.path()
+            Cow::Borrowed(path)
         }
     }
 
     /// Returns the path as it should be presented to the user.
     pub fn into_stripped_path(self, config: &Config) -> PathBuf {
         if config.strip_cwd_prefix {
-            self.stripped_path(config).to_path_buf()
+            self.stripped_path(config).into_owned()
         } else {
             self.into_path()
         }
@@ -101,6 +110,10 @@ impl DirEntry {
     }
 }
 
+fn starts_with_dash(path: &Path) -> bool {
+    path.as_os_str().as_encoded_bytes().first() == Some(&b'-')
+}
+
 impl PartialEq for DirEntry {
     #[inline]
     fn eq(&self, other: &Self) -> bool {
@@ -153,3 +166,25 @@ impl Colorable for DirEntry {
         self.metadata().cloned()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::starts_with_dash;
+    use std::path::Path;
+
+    #[test]
+    fn dash_prefixed_paths_detected() {
+        assert!(starts_with_dash(Path::new("-rf")));
+        assert!(starts_with_dash(Path::new("--delete")));
+        assert!(starts_with_dash(Path::new("-")));
+    }
+
+    #[test]
+    fn safe_paths_not_flagged() {
+        assert!(!starts_with_dash(Path::new("foo")));
+        assert!(!starts_with_dash(Path::new("./foo")));
+        assert!(!starts_with_dash(Path::new("sub/-rf")));
+        assert!(!starts_with_dash(Path::new("")));
+        assert!(!starts_with_dash(Path::new(" -rf")));
+    }
+}

src/exec/job.rs

@@ -32,7 +32,7 @@ pub fn job(
 
         // Generate a command, execute it and store its exit code.
         let code = cmd.execute(
-            dir_entry.stripped_path(config),
+            &dir_entry.stripped_path(config),
             config.path_separator.as_deref(),
             config.null_separator,
             buffer_output,

src/exec/mod.rs

@@ -175,7 +175,7 @@ impl CommandBuilder {
             self.finish()?;
         }
 
-        let arg = self.path_arg.generate_for_exec(path, separator);
+        let arg = self.path_arg.generate(path, separator);
         if !self
             .cmd
             .args_would_fit(iter::once(&arg).chain(&self.post_args))
@@ -242,8 +242,6 @@ impl CommandTemplate {
             bail!("No executable provided for --exec or --exec-batch");
         }
 
-        // The first argument is the command to execute and must be a fixed string;
-        // allowing a placeholder here would turn every matched file into an execve target.
         if args[0].has_tokens() {
             bail!("First argument of --exec/--exec-batch must be a fixed executable, not a placeholder");
         }
@@ -265,9 +263,9 @@ impl CommandTemplate {
     /// Using the internal `args` field, and a supplied `input` variable, a `Command` will be
     /// build.
     fn generate(&self, input: &Path, path_separator: Option<&str>) -> io::Result<Command> {
-        let mut cmd = Command::new(self.args[0].generate_for_exec(input, path_separator));
+        let mut cmd = Command::new(self.args[0].generate(input, path_separator));
         for arg in &self.args[1..] {
-            cmd.try_arg(arg.generate_for_exec(input, path_separator))?;
+            cmd.try_arg(arg.generate(input, path_separator))?;
         }
         Ok(cmd)
     }

src/fmt/mod.rs

@@ -24,19 +24,6 @@ pub enum Token {
     Text(String),
 }
 
-impl Token {
-    fn is_path_derived(&self) -> bool {
-        matches!(
-            self,
-            Token::Placeholder
-                | Token::Basename
-                | Token::Parent
-                | Token::NoExt
-                | Token::BasenameNoExt
-        )
-    }
-}
-
 impl Display for Token {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         match *self {
@@ -123,26 +110,8 @@ impl FormatTemplate {
     /// the path separator in all placeholder tokens. Fixed text and tokens are not affected by
     /// path separator substitution.
     pub fn generate(&self, path: impl AsRef<Path>, path_separator: Option<&str>) -> OsString {
-        self.generate_impl(path.as_ref(), path_separator, false)
-    }
-
-    /// Like `generate`, but prepends `./` when a path-derived token produces a leading `-`,
-    /// so the target of `--exec` does not see it as an option.
-    pub fn generate_for_exec(
-        &self,
-        path: impl AsRef<Path>,
-        path_separator: Option<&str>,
-    ) -> OsString {
-        self.generate_impl(path.as_ref(), path_separator, true)
-    }
-
-    fn generate_impl(
-        &self,
-        path: &Path,
-        path_separator: Option<&str>,
-        protect_leading_dash: bool,
-    ) -> OsString {
         use Token::*;
+        let path = path.as_ref();
 
         match *self {
             Self::Tokens(ref tokens) => {
@@ -165,16 +134,6 @@ impl FormatTemplate {
                         Text(string) => s.push(string),
                     }
                 }
-
-                if protect_leading_dash
-                    && tokens.first().is_some_and(|t| t.is_path_derived())
-                    && s.as_encoded_bytes().first() == Some(&b'-')
-                {
-                    let mut prefixed = OsString::from("./");
-                    prefixed.push(s);
-                    return prefixed;
-                }
-
                 s
             }
             Self::Text(ref text) => OsString::from(text),
@@ -319,86 +278,4 @@ mod fmt_tests {
             basenameNoExt=file }"
         );
     }
-
-    fn tmpl(s: &str) -> FormatTemplate {
-        FormatTemplate::parse(s)
-    }
-
-    #[test]
-    fn exec_basename_with_leading_dash_is_guarded() {
-        let out = tmpl("{/}")
-            .generate_for_exec(Path::new("./some/dir/-rf"), None)
-            .into_string()
-            .unwrap();
-        assert_eq!(out, "./-rf");
-    }
-
-    #[test]
-    fn exec_basename_no_ext_with_leading_dash_is_guarded() {
-        let out = tmpl("{/.}")
-            .generate_for_exec(Path::new("./some/dir/-evil.txt"), None)
-            .into_string()
-            .unwrap();
-        assert_eq!(out, "./-evil");
-    }
-
-    #[test]
-    fn exec_parent_with_leading_dash_is_guarded() {
-        let out = tmpl("{//}")
-            .generate_for_exec(Path::new("-startdir/inner/file.txt"), None)
-            .into_string()
-            .unwrap();
-        assert_eq!(out, "./-startdir/inner");
-    }
-
-    #[test]
-    fn exec_plain_placeholder_already_safe() {
-        let out = tmpl("{}")
-            .generate_for_exec(Path::new("./some/dir/-rf"), None)
-            .into_string()
-            .unwrap();
-        assert_eq!(out, "./some/dir/-rf");
-    }
-
-    #[test]
-    fn exec_user_literal_dash_prefix_not_rewritten() {
-        let out = tmpl("-{/}")
-            .generate_for_exec(Path::new("./some/dir/normal.txt"), None)
-            .into_string()
-            .unwrap();
-        assert_eq!(out, "-normal.txt");
-    }
-
-    #[test]
-    fn exec_user_literal_dash_before_dash_basename() {
-        let out = tmpl("-{/}")
-            .generate_for_exec(Path::new("./some/dir/-name"), None)
-            .into_string()
-            .unwrap();
-        assert_eq!(out, "--name");
-    }
-
-    #[test]
-    fn exec_suffix_after_placeholder_preserves_guard() {
-        let out = tmpl("{/}.bak")
-            .generate_for_exec(Path::new("./some/dir/-foo"), None)
-            .into_string()
-            .unwrap();
-        assert_eq!(out, "./-foo.bak");
-    }
-
-    #[test]
-    fn format_mode_does_not_protect_leading_dash() {
-        let out = tmpl("{/}")
-            .generate(Path::new("./some/dir/-rf"), None)
-            .into_string()
-            .unwrap();
-        assert_eq!(out, "-rf");
-    }
-
-    #[test]
-    fn exec_empty_result_not_prefixed() {
-        let out = tmpl("{/}").generate_for_exec(Path::new(""), None);
-        assert!(out.is_empty());
-    }
 }

src/main.rs

@@ -66,13 +66,7 @@ fn main() {
             exit_code.exit();
         }
         Err(err) => {
-            // Gap fix: anyhow errors bubbled from run() may embed user paths/args.
-            let formatted = format!("{err:#}");
-            if std::io::stderr().is_terminal() {
-                eprintln!("[fd error]: {}", sanitize::sanitize_for_terminal(&formatted));
-            } else {
-                eprintln!("[fd error]: {formatted}");
-            }
+            crate::error::print_error(format!("{err:#}"));
             ExitCode::GeneralError.exit();
         }
     }

src/sanitize.rs

@@ -1,24 +1,29 @@
 //! TTY-output sanitization to prevent terminal escape injection via filenames.
 
 use std::borrow::Cow;
+use std::fmt::Write;
 
 #[inline]
 fn is_dangerous_control(c: char) -> bool {
-    // C0 (except HT), DEL, and C1 controls (gap fix: U+0080..=U+009F can act
-    // as single-byte CSI/OSC initiators on 8-bit-control terminals).
+    // C0 (except HT), DEL, and C1 controls (U+0080..=U+009F can act as
+    // single-byte CSI/OSC initiators on 8-bit-control terminals).
     matches!(c, '\x00'..='\x08' | '\x0A'..='\x1F' | '\x7F' | '\u{80}'..='\u{9F}')
 }
 
-/// Replace control characters with `?` (matches `ls -q`). Borrows on the fast path.
+/// Replace control characters with `\xNN` so the original filename remains recoverable.
 pub fn sanitize_for_terminal(s: &str) -> Cow<'_, str> {
     if !s.chars().any(is_dangerous_control) {
         return Cow::Borrowed(s);
     }
-    Cow::Owned(
-        s.chars()
-            .map(|c| if is_dangerous_control(c) { '?' } else { c })
-            .collect(),
-    )
+    let mut out = String::with_capacity(s.len());
+    for c in s.chars() {
+        if is_dangerous_control(c) {
+            let _ = write!(out, "\\x{:02X}", c as u32);
+        } else {
+            out.push(c);
+        }
+    }
+    Cow::Owned(out)
 }
 
 #[cfg(test)]
@@ -27,7 +32,6 @@ mod tests {
 
     #[test]
     fn preserves_safe_content() {
-        // Plain text, unicode, tab, and existing U+FFFD all pass through without allocation.
         for s in ["hello.txt", "résumé.pdf", "文档.txt", "🦀.rs", "a\tb", "a\u{FFFD}b"] {
             assert!(matches!(sanitize_for_terminal(s), Cow::Borrowed(_)), "{s:?}");
             assert_eq!(sanitize_for_terminal(s), s);
@@ -39,12 +43,12 @@ mod tests {
         let attack = "innocent\x1b]52;c;cHduZWQ=\x1b\\.txt";
         let safe = sanitize_for_terminal(attack);
         assert!(!safe.contains('\x1b'));
-        assert_eq!(safe, "innocent?]52;c;cHduZWQ=?\\.txt");
+        assert_eq!(safe, "innocent\\x1B]52;c;cHduZWQ=\\x1B\\.txt");
     }
 
     #[test]
     fn strips_cr_output_forgery() {
-        assert_eq!(sanitize_for_terminal("A\rFAKE OUTPUT"), "A?FAKE OUTPUT");
+        assert_eq!(sanitize_for_terminal("A\rFAKE OUTPUT"), "A\\x0DFAKE OUTPUT");
     }
 
     #[test]
@@ -55,24 +59,30 @@ mod tests {
 
     #[test]
     fn strips_del() {
-        assert_eq!(sanitize_for_terminal("a\x7fb"), "a?b");
+        assert_eq!(sanitize_for_terminal("a\x7fb"), "a\\x7Fb");
     }
 
     #[test]
     fn strips_bel_and_null() {
-        assert_eq!(sanitize_for_terminal("a\x07b"), "a?b");
-        assert_eq!(sanitize_for_terminal("a\0b"), "a?b");
+        assert_eq!(sanitize_for_terminal("a\x07b"), "a\\x07b");
+        assert_eq!(sanitize_for_terminal("a\0b"), "a\\x00b");
     }
 
     #[test]
     fn strips_newline() {
-        assert_eq!(sanitize_for_terminal("a\nb"), "a?b");
+        assert_eq!(sanitize_for_terminal("a\nb"), "a\\x0Ab");
+    }
+
+    #[test]
+    fn escape_preserves_information() {
+        let s = "name\x1bX\x07Y.txt";
+        assert_eq!(sanitize_for_terminal(s), "name\\x1BX\\x07Y.txt");
     }
 
     #[test]
     fn strips_c1_csi_and_osc_initiators() {
         // U+009B is CSI, U+009D is OSC; dangerous on 8-bit-control terminals.
-        assert_eq!(sanitize_for_terminal("\u{9b}31m"), "?31m");
-        assert_eq!(sanitize_for_terminal("\u{9d}0;pwned\u{9c}"), "?0;pwned?");
+        assert_eq!(sanitize_for_terminal("\u{9b}31m"), "\\x9B31m");
+        assert_eq!(sanitize_for_terminal("\u{9d}0;pwned\u{9c}"), "\\x9D0;pwned\\x9C");
     }
 }

tests/tests.rs

@@ -1906,7 +1906,7 @@ fn test_exec_batch() {
 
         te.assert_failure_with_error(
             &["foo", "--exec-batch", "echo {}"],
-            "error: First argument of exec-batch is expected to be a fixed executable\n\
+            "error: First argument of --exec/--exec-batch must be a fixed executable, not a placeholder\n\
             \n\
             Usage: fd [OPTIONS] [pattern] [path]...\n\
             \n\