mktemp: handle invalid UTF-8 in suffix gracefully (#10818)

Author: sylvestre

src/uu/mktemp/src/mktemp.rs

@@ -101,7 +101,7 @@ pub struct Options {
     pub tmpdir: Option<PathBuf>,
 
     /// The suffix to append to the temporary file, if any.
-    pub suffix: Option<String>,
+    pub suffix: Option<OsString>,
 
     /// Whether to treat the template argument as a single file path component.
     pub treat_as_template: bool,
@@ -150,7 +150,7 @@ impl Options {
             dry_run: matches.get_flag(OPT_DRY_RUN),
             quiet: matches.get_flag(OPT_QUIET),
             tmpdir,
-            suffix: matches.get_one::<String>(OPT_SUFFIX).map(String::from),
+            suffix: matches.get_one::<OsString>(OPT_SUFFIX).cloned(),
             treat_as_template: matches.get_flag(OPT_T),
             template,
         }
@@ -214,27 +214,39 @@ fn find_last_contiguous_block_of_xs(s: &str) -> Option<(usize, usize)> {
 impl Params {
     fn from(options: Options) -> Result<Self, MkTempError> {
         // Convert OsString template to string for processing
-        let Some(template_str) = options.template.to_str() else {
-            // For non-UTF-8 templates, return an error
-            return Err(MkTempError::InvalidTemplate(options.template));
+        // When using -t flag, be permissive with invalid UTF-8 like GNU mktemp
+        // Otherwise, maintain strict UTF-8 validation (existing behavior)
+        let template_str = if options.treat_as_template {
+            // For -t templates, use lossy conversion for GNU compatibility
+            options.template.to_string_lossy().into_owned()
+        } else {
+            // For regular templates, maintain strict validation
+            match options.template.to_str() {
+                Some(s) => s.to_string(),
+                None => {
+                    return Err(MkTempError::InvalidTemplate(
+                        "template contains invalid UTF-8".into(),
+                    ));
+                }
+            }
         };
 
         // The template argument must end in 'X' if a suffix option is given.
         if options.suffix.is_some() && !template_str.ends_with('X') {
-            return Err(MkTempError::MustEndInX(template_str.to_string()));
+            return Err(MkTempError::MustEndInX(template_str.clone()));
         }
 
         // Get the start and end indices of the randomized part of the template.
         //
         // For example, if the template is "abcXXXXyz", then `i` is 3 and `j` is 7.
-        let Some((i, j)) = find_last_contiguous_block_of_xs(template_str) else {
+        let Some((i, j)) = find_last_contiguous_block_of_xs(&template_str) else {
             let s = match options.suffix {
                 // If a suffix is specified, the error message includes the template without the suffix.
                 Some(_) => template_str
                     .chars()
                     .take(template_str.len())
                     .collect::<String>(),
-                None => template_str.to_string(),
+                None => template_str.clone(),
             };
             return Err(MkTempError::TooFewXs(s));
         };
@@ -249,11 +261,11 @@ impl Params {
         let prefix_path = Path::new(&prefix_from_option).join(prefix_from_template);
         if options.treat_as_template && prefix_from_template.contains(MAIN_SEPARATOR) {
             return Err(MkTempError::PrefixContainsDirSeparator(
-                template_str.to_string(),
+                template_str.clone(),
             ));
         }
         if tmpdir.is_some() && Path::new(prefix_from_template).is_absolute() {
-            return Err(MkTempError::InvalidTemplate(template_str.into()));
+            return Err(MkTempError::InvalidTemplate(template_str.clone().into()));
         }
 
         // Split the parent directory from the file part of the prefix.
@@ -271,7 +283,7 @@ impl Params {
                 };
                 let prefix = match prefix_path.file_name() {
                     None => String::new(),
-                    Some(f) => f.to_str().unwrap().to_string(),
+                    Some(f) => f.to_string_lossy().to_string(),
                 };
                 (directory, prefix)
             }
@@ -281,7 +293,10 @@ impl Params {
         //
         // For example, if the suffix command-line argument is ".txt" and
         // the template is "XXXabc", then `suffix` is "abc.txt".
-        let suffix_from_option = options.suffix.unwrap_or_default();
+        let suffix_from_option = options
+            .suffix
+            .map(|s| s.to_string_lossy().to_string())
+            .unwrap_or_default();
         let suffix_from_template = &template_str[j..];
         let suffix = format!("{suffix_from_template}{suffix_from_option}");
         if suffix.contains(MAIN_SEPARATOR) {
@@ -445,7 +460,8 @@ pub fn uu_app() -> Command {
             Arg::new(OPT_SUFFIX)
                 .long(OPT_SUFFIX)
                 .help(translate!("mktemp-help-suffix"))
-                .value_name("SUFFIX"),
+                .value_name("SUFFIX")
+                .value_parser(clap::value_parser!(OsString)),
         )
         .arg(
             Arg::new(OPT_P)

tests/by-util/test_mktemp.rs

@@ -1139,6 +1139,26 @@ fn test_non_utf8_tmpdir_long_option() {
         .succeeds();
 }
 
+#[test]
+#[cfg(target_os = "linux")]
+fn test_invalid_utf8_suffix() {
+    use std::os::unix::ffi::OsStrExt;
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    // Create invalid UTF-8 bytes for suffix
+    // This mimics the GNU test which tests mktemp with bad unicode characters
+    let invalid_utf8 = std::ffi::OsStr::from_bytes(b"\xC3|\xED\xBA\xAD");
+
+    // Test that mktemp handles invalid UTF-8 in suffix gracefully
+    // It should succeed and create a file with the lossy conversion of the invalid UTF-8
+    ucmd.arg("-p")
+        .arg(at.as_string())
+        .arg("--suffix")
+        .arg(invalid_utf8)
+        .arg("tmpXXXXXX")
+        .succeeds();
+}
+
 #[test]
 #[cfg(target_os = "linux")]
 fn test_non_utf8_tmpdir_directory_creation() {