From 0706675830e599b5dcafdb64b77a450a80927ae8 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 8 Aug 2025 14:52:14 +0200 Subject: [PATCH] Fix mktemp to handle non-UTF-8 templates --- src/uu/mktemp/src/mktemp.rs | 44 +++++++++++++++++++++--------------- tests/by-util/test_mktemp.rs | 14 ++++++++++++ 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index 23d22d361..b9cb58289 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -13,7 +13,7 @@ use uucore::format_usage; use uucore::translate; use std::env; -use std::ffi::OsStr; +use std::ffi::{OsStr, OsString}; use std::io::ErrorKind; use std::iter; use std::path::{MAIN_SEPARATOR, Path, PathBuf}; @@ -105,7 +105,7 @@ pub struct Options { pub treat_as_template: bool, /// The template to use for the name of the temporary file. - pub template: String, + pub template: OsString, } impl Options { @@ -123,12 +123,12 @@ impl Options { .ok() .map_or_else(env::temp_dir, PathBuf::from), }); - let (tmpdir, template) = match matches.get_one::(ARG_TEMPLATE) { + let (tmpdir, template) = match matches.get_one::(ARG_TEMPLATE) { // If no template argument is given, `--tmpdir` is implied. None => { let tmpdir = Some(tmpdir.unwrap_or_else(env::temp_dir)); let template = DEFAULT_TEMPLATE; - (tmpdir, template.to_string()) + (tmpdir, OsString::from(template)) } Some(template) => { let tmpdir = if env::var(TMPDIR_ENV_VAR).is_ok() && matches.get_flag(OPT_T) { @@ -142,7 +142,7 @@ impl Options { } else { None }; - (tmpdir, template.to_string()) + (tmpdir, template.clone()) } }; Self { @@ -200,23 +200,31 @@ fn find_last_contiguous_block_of_xs(s: &str) -> Option<(usize, usize)> { impl Params { fn from(options: Options) -> Result { + // Convert OsString template to string for processing + let template_str = match options.template.to_str() { + Some(s) => s, + None => { + // For non-UTF-8 templates, return an error + return Err(MkTempError::InvalidTemplate(options.template.to_string_lossy().into_owned())); + } + }; + // The template argument must end in 'X' if a suffix option is given. - if options.suffix.is_some() && !options.template.ends_with('X') { - return Err(MkTempError::MustEndInX(options.template)); + if options.suffix.is_some() && !template_str.ends_with('X') { + return Err(MkTempError::MustEndInX(template_str.to_string())); } // 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(&options.template) 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(_) => options - .template + Some(_) => template_str .chars() - .take(options.template.len()) + .take(template_str.len()) .collect::(), - None => options.template, + None => template_str.to_string(), }; return Err(MkTempError::TooFewXs(s)); }; @@ -227,16 +235,16 @@ impl Params { // then `prefix` is "a/b/c/d". let tmpdir = options.tmpdir; let prefix_from_option = tmpdir.clone().unwrap_or_default(); - let prefix_from_template = &options.template[..i]; + let prefix_from_template = &template_str[..i]; let prefix = Path::new(&prefix_from_option) .join(prefix_from_template) .display() .to_string(); if options.treat_as_template && prefix_from_template.contains(MAIN_SEPARATOR) { - return Err(MkTempError::PrefixContainsDirSeparator(options.template)); + return Err(MkTempError::PrefixContainsDirSeparator(template_str.to_string())); } if tmpdir.is_some() && Path::new(prefix_from_template).is_absolute() { - return Err(MkTempError::InvalidTemplate(options.template)); + return Err(MkTempError::InvalidTemplate(template_str.to_string())); } // Split the parent directory from the file part of the prefix. @@ -263,7 +271,7 @@ 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_template = &options.template[j..]; + let suffix_from_template = &template_str[j..]; let suffix = format!("{suffix_from_template}{suffix_from_option}"); if suffix.contains(MAIN_SEPARATOR) { return Err(MkTempError::SuffixContainsDirSeparator(suffix)); @@ -360,7 +368,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // If POSIXLY_CORRECT was set, template MUST be the last argument. if matches.contains_id(ARG_TEMPLATE) { // Template argument was provided, check if was the last one. - if args.last().unwrap() != OsStr::new(&options.template) { + if args.last().unwrap() != &options.template { return Err(Box::new(MkTempError::TooManyTemplates)); } } @@ -457,7 +465,7 @@ pub fn uu_app() -> Command { .help(translate!("mktemp-help-t")) .action(ArgAction::SetTrue), ) - .arg(Arg::new(ARG_TEMPLATE).num_args(..=1)) + .arg(Arg::new(ARG_TEMPLATE).num_args(..=1).value_parser(clap::value_parser!(OsString))) } fn dry_exec(tmpdir: &Path, prefix: &str, rand: usize, suffix: &str) -> UResult { diff --git a/tests/by-util/test_mktemp.rs b/tests/by-util/test_mktemp.rs index 60714ad7f..872924eb3 100644 --- a/tests/by-util/test_mktemp.rs +++ b/tests/by-util/test_mktemp.rs @@ -997,3 +997,17 @@ fn test_missing_short_tmpdir_flag() { .no_stdout() .stderr_contains("a value is required for '-p ' but none was supplied"); } + +#[test] +#[cfg(target_os = "linux")] +fn test_non_utf8_template() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let ts = TestScenario::new(util_name!()); + + // Test that mktemp gracefully handles non-UTF-8 templates with an error instead of panicking + let template = OsStr::from_bytes(b"test_\xFF\xFE_XXXXXX"); + + ts.ucmd().arg(template).fails().stderr_contains("invalid"); +}