From c5d31fcf01015e4de69db3b5d2d12c387cebac0b Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sat, 15 Feb 2025 15:12:52 -0800 Subject: [PATCH 01/11] feat: update MSRV from 1.63 to 1.84 --- .github/workflows/ci.yml | 2 +- Cargo.toml | 2 +- README.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e300a66bd..063dc2e02 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: rust-version: - nightly - stable - - "1.63" + - "1.84" os: - ubuntu-latest - windows-latest diff --git a/Cargo.toml b/Cargo.toml index 84d3540a6..f76065ce4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,7 @@ authors = [ ] documentation = "https://docs.rs/tempfile" edition = "2021" -rust-version = "1.63" +rust-version = "1.84" homepage = "https://stebalien.com/projects/tempfile-rs/" keywords = ["tempfile", "tmpfile", "filesystem"] license = "MIT OR Apache-2.0" diff --git a/README.md b/README.md index 4d886b135..1b1c0fbd9 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ patterns and surprisingly difficult to implement securely). Usage ----- -Minimum required Rust version: 1.63.0 +Minimum required Rust version: 1.84.0 Add this to your `Cargo.toml`: From 0bd3c4cb817ff8bdaa1530f6c1c78de9cd9f411b Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 10:52:16 +0700 Subject: [PATCH 02/11] fix: implement AsRef by-reference Otherwise it's really easy to pass a temporary file handle by reference, causing it to be dropped by the receiver deleting the underlying file. fixes #115 --- src/dir/mod.rs | 5 ++++- src/file/mod.rs | 6 +++--- tests/namedtempfile.rs | 8 +++++++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/dir/mod.rs b/src/dir/mod.rs index 4b0c37af9..3e2a156a5 100644 --- a/src/dir/mod.rs +++ b/src/dir/mod.rs @@ -467,7 +467,10 @@ impl TempDir { } } -impl AsRef for TempDir { +// NOTE: This is implemented on &TempDir, not TempDir, to prevent accidentally moving the TempDir +// into a function that calls `as_ref()` before immediately dropping it (deleting the underlying +// temporary directory). +impl AsRef for &TempDir { fn as_ref(&self) -> &Path { self.path() } diff --git a/src/file/mod.rs b/src/file/mod.rs index 4eb6b6755..eb005b9f6 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -355,13 +355,13 @@ impl Deref for TempPath { } } -impl AsRef for TempPath { +impl AsRef for &TempPath { fn as_ref(&self) -> &Path { &self.path } } -impl AsRef for TempPath { +impl AsRef for &TempPath { fn as_ref(&self) -> &OsStr { self.path.as_os_str() } @@ -445,7 +445,7 @@ impl fmt::Debug for NamedTempFile { } } -impl AsRef for NamedTempFile { +impl AsRef for &NamedTempFile { #[inline] fn as_ref(&self) -> &Path { self.path() diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs index 2e6446627..88522266a 100644 --- a/tests/namedtempfile.rs +++ b/tests/namedtempfile.rs @@ -277,7 +277,13 @@ fn temp_path_from_argument_types() { #[test] fn test_write_after_close() { let path = NamedTempFile::new().unwrap().into_temp_path(); - File::create(path).unwrap().write_all(b"test").unwrap(); + let mut f = File::options() + .read(true) + .write(true) + .create(false) + .open(&path) + .unwrap(); + f.write_all(b"test").unwrap(); } #[test] From e7d10d7f26067ff1af9428567f41516fe11401ff Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 10:54:36 +0700 Subject: [PATCH 03/11] feat: forward Seek::rewind --- src/file/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/file/mod.rs b/src/file/mod.rs index eb005b9f6..bd50b5298 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -998,12 +998,19 @@ impl Seek for NamedTempFile { fn seek(&mut self, pos: SeekFrom) -> io::Result { self.as_file_mut().seek(pos).with_err_path(|| self.path()) } + fn rewind(&mut self) -> io::Result<()> { + self.as_file_mut().rewind().with_err_path(|| self.path()) + } } impl Seek for &NamedTempFile { fn seek(&mut self, pos: SeekFrom) -> io::Result { self.as_file().seek(pos).with_err_path(|| self.path()) } + + fn rewind(&mut self) -> io::Result<()> { + self.as_file().rewind().with_err_path(|| self.path()) + } } #[cfg(any(unix, target_os = "wasi"))] From f4c8547dd3af15b5c1352c0eb5d2c92fbd58bbae Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 10:54:52 +0700 Subject: [PATCH 04/11] fix: create temporary directories with 0700 permissions by default fixes #284 --- src/dir/imp/unix.rs | 4 +--- src/lib.rs | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/dir/imp/unix.rs b/src/dir/imp/unix.rs index d69dac389..36076e757 100644 --- a/src/dir/imp/unix.rs +++ b/src/dir/imp/unix.rs @@ -12,9 +12,7 @@ pub fn create( #[cfg(not(target_os = "wasi"))] { use std::os::unix::fs::{DirBuilderExt, PermissionsExt}; - if let Some(p) = permissions { - dir_options.mode(p.mode()); - } + dir_options.mode(permissions.map(|p| p.mode()).unwrap_or(0o700)); } dir_options .create(&path) diff --git a/src/lib.rs b/src/lib.rs index f5665a6c9..122292077 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -407,9 +407,9 @@ impl<'a, 'b> Builder<'a, 'b> { /// applied by the underlying syscall. The actual permission bits are calculated via /// `permissions & !umask`. /// - /// Permissions default to `0o600` for tempfiles and `0o777` for tempdirs. Note, this doesn't - /// include effects of the current `umask`. For example, combined with the standard umask - /// `0o022`, the defaults yield `0o600` for tempfiles and `0o755` for tempdirs. + /// Permissions default to `0o600` for temporary files and `0o700` for temporary directories. + /// Note, this doesn't include effects of the current `umask`. For example, combined with the + /// standard umask `0o022`, the defaults yield `0o600` for files and `0o700` for directories. /// /// ## Windows and others /// From 8242a23caea6bd2f921e85717c2f2417a0eca23a Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 10:58:13 +0700 Subject: [PATCH 05/11] feat: misc builder improvements and better defaults 1. Default to "tmp" as the file prefix instead of ".tmp". 2. Make it possible to construct the builder in a constfn. 3. Use a single lifetime in the builder. --- src/lib.rs | 64 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 122292077..72a5e179a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -197,9 +197,11 @@ doc_comment::doctest!("../README.md"); const NUM_RETRIES: u32 = 65536; const NUM_RAND_CHARS: usize = 6; +const DEFAULT_PREFIX: &str = "tmp"; +const DEFAULT_SUFFIX: &str = ""; use std::ffi::OsStr; -use std::fs::OpenOptions; +use std::fs::{OpenOptions, Permissions}; use std::io; use std::path::Path; @@ -219,29 +221,22 @@ pub use crate::spooled::{spooled_tempfile, SpooledData, SpooledTempFile}; /// Create a new temporary file or directory with custom options. #[derive(Debug, Clone, Eq, PartialEq)] -pub struct Builder<'a, 'b> { +pub struct Builder<'a> { random_len: usize, - prefix: &'a OsStr, - suffix: &'b OsStr, + prefix: Option<&'a OsStr>, + suffix: Option<&'a OsStr>, append: bool, - permissions: Option, + permissions: Option, keep: bool, } -impl Default for Builder<'_, '_> { +impl Default for Builder<'_> { fn default() -> Self { - Builder { - random_len: crate::NUM_RAND_CHARS, - prefix: OsStr::new(".tmp"), - suffix: OsStr::new(""), - append: false, - permissions: None, - keep: false, - } + Builder::new() } } -impl<'a, 'b> Builder<'a, 'b> { +impl<'a> Builder<'a> { /// Create a new `Builder`. /// /// # Examples @@ -308,14 +303,21 @@ impl<'a, 'b> Builder<'a, 'b> { /// # Ok::<(), std::io::Error>(()) /// ``` #[must_use] - pub fn new() -> Self { - Self::default() + pub const fn new() -> Self { + Builder { + random_len: crate::NUM_RAND_CHARS, + prefix: None, + suffix: None, + append: false, + permissions: None, + keep: false, + } } /// Set a custom filename prefix. /// /// Path separators are legal but not advisable. - /// Default: `.tmp`. + /// Default: `tmp`. /// /// # Examples /// @@ -328,7 +330,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// # Ok::<(), std::io::Error>(()) /// ``` pub fn prefix + ?Sized>(&mut self, prefix: &'a S) -> &mut Self { - self.prefix = prefix.as_ref(); + self.prefix = Some(prefix.as_ref()); self } @@ -347,8 +349,8 @@ impl<'a, 'b> Builder<'a, 'b> { /// .tempfile()?; /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn suffix + ?Sized>(&mut self, suffix: &'b S) -> &mut Self { - self.suffix = suffix.as_ref(); + pub fn suffix + ?Sized>(&mut self, suffix: &'a S) -> &mut Self { + self.suffix = Some(suffix.as_ref()); self } @@ -366,7 +368,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// .tempfile()?; /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn rand_bytes(&mut self, rand: usize) -> &mut Self { + pub const fn rand_bytes(&mut self, rand: usize) -> &mut Self { self.random_len = rand; self } @@ -385,7 +387,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// .tempfile()?; /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn append(&mut self, append: bool) -> &mut Self { + pub const fn append(&mut self, append: bool) -> &mut Self { self.append = append; self } @@ -457,7 +459,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// # } /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn permissions(&mut self, permissions: std::fs::Permissions) -> &mut Self { + pub const fn permissions(&mut self, permissions: std::fs::Permissions) -> &mut Self { self.permissions = Some(permissions); self } @@ -481,7 +483,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// .tempfile()?; /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn keep(&mut self, keep: bool) -> &mut Self { + pub const fn keep(&mut self, keep: bool) -> &mut Self { self.keep = keep; self } @@ -543,8 +545,8 @@ impl<'a, 'b> Builder<'a, 'b> { pub fn tempfile_in>(&self, dir: P) -> io::Result { util::create_helper( dir.as_ref(), - self.prefix, - self.suffix, + self.prefix.unwrap_or(DEFAULT_PREFIX.as_ref()), + self.suffix.unwrap_or(DEFAULT_SUFFIX.as_ref()), self.random_len, |path| { file::create_named( @@ -609,8 +611,8 @@ impl<'a, 'b> Builder<'a, 'b> { pub fn tempdir_in>(&self, dir: P) -> io::Result { util::create_helper( dir.as_ref(), - self.prefix, - self.suffix, + self.prefix.unwrap_or(DEFAULT_PREFIX.as_ref()), + self.suffix.unwrap_or(DEFAULT_SUFFIX.as_ref()), self.random_len, |path| dir::create(path, self.permissions.as_ref(), self.keep), ) @@ -731,8 +733,8 @@ impl<'a, 'b> Builder<'a, 'b> { { util::create_helper( dir.as_ref(), - self.prefix, - self.suffix, + self.prefix.unwrap_or(DEFAULT_PREFIX.as_ref()), + self.suffix.unwrap_or(DEFAULT_SUFFIX.as_ref()), self.random_len, move |path| { Ok(NamedTempFile::from_parts( From 80bbdb6247f3ea8752941d1ebe43abb1e3ef412f Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 11:00:35 +0700 Subject: [PATCH 06/11] fix: forbid path separators in prefix/suffix fixes #59 --- src/lib.rs | 4 ++-- src/util.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 72a5e179a..9cfaa3dbe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -316,7 +316,7 @@ impl<'a> Builder<'a> { /// Set a custom filename prefix. /// - /// Path separators are legal but not advisable. + /// Path separators are not allowed. /// Default: `tmp`. /// /// # Examples @@ -336,7 +336,7 @@ impl<'a> Builder<'a> { /// Set a custom filename suffix. /// - /// Path separators are legal but not advisable. + /// Path separators are not allowed. /// Default: empty. /// /// # Examples diff --git a/src/util.rs b/src/util.rs index 5dc62a291..cea17bb79 100644 --- a/src/util.rs +++ b/src/util.rs @@ -4,7 +4,12 @@ use std::{io, iter::repeat_with}; use crate::error::IoResultExt; -fn tmpname(rng: &mut fastrand::Rng, prefix: &OsStr, suffix: &OsStr, rand_len: usize) -> OsString { +fn tmpname( + rng: &mut fastrand::Rng, + prefix: &OsStr, + suffix: &OsStr, + rand_len: usize, +) -> io::Result { let capacity = prefix .len() .saturating_add(suffix.len()) @@ -16,7 +21,10 @@ fn tmpname(rng: &mut fastrand::Rng, prefix: &OsStr, suffix: &OsStr, rand_len: us buf.push(c.encode_utf8(&mut char_buf)); } buf.push(suffix); - buf + + check_valid_filename(&buf)?; + + Ok(buf) } pub fn create_helper( @@ -62,7 +70,7 @@ pub fn create_helper( rng.seed(seed); } } - let path = base.join(tmpname(&mut rng, prefix, suffix, random_len)); + let path = base.join(tmpname(&mut rng, prefix, suffix, random_len)?); return match f(path) { Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists && num_retries > 1 => continue, // AddrInUse can happen if we're creating a UNIX domain socket and @@ -78,3 +86,47 @@ pub fn create_helper( )) .with_err_path(|| base) } + +/// Check that the passed path is a valid file-name (no directories, no nulls, etc.). +fn check_valid_filename(path: impl AsRef) -> io::Result<()> { + let path = path.as_ref(); + + // Make sure the filename isn't empty. + if path.is_empty() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "temporary filename is empty", + )); + } + + // Check for null bytes, encoding doesn't matter. The OS/libc will do this for us, but we might + // as well be extra safe. + if path.as_encoded_bytes().contains(&0) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("temporary filename {path:?} contains a null byte"), + )); + } + + // Make sure the filename is exactly one path element and make sure that element is a file name. + // This is the most reliable way to check for path separators. + if Path::new(path).file_name() != Some(path) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("temporary filename {path:?} is not a valid path component"), + )); + } + + Ok(()) +} + +#[test] +fn test_filename_validation() { + check_valid_filename("foo").unwrap(); + check_valid_filename("foo.bar").unwrap(); + check_valid_filename("/foo").expect_err("path contains a slash"); + check_valid_filename("foo/bar").expect_err("path contains a slash"); + check_valid_filename("foo/").expect_err("path contains a slash"); + check_valid_filename("/").expect_err("path contains a slash"); + check_valid_filename("\0").expect_err("path contains a null"); +} From fba8d07c7e4552f1316d078152686efe2cff8e64 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 12:24:33 +0700 Subject: [PATCH 07/11] feat: change Builder::make to take a parameters struct That way, we can pass through additional builder options. This new struct implements AsRef and Deref so breakage should be minimal. --- src/lib.rs | 65 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 50 insertions(+), 15 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9cfaa3dbe..b33b439d6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -203,6 +203,7 @@ const DEFAULT_SUFFIX: &str = ""; use std::ffi::OsStr; use std::fs::{OpenOptions, Permissions}; use std::io; +use std::ops::Deref; use std::path::Path; mod dir; @@ -619,11 +620,13 @@ impl<'a> Builder<'a> { } /// Attempts to create a temporary file (or file-like object) using the - /// provided closure. The closure is passed a temporary file path and - /// returns an [`std::io::Result`]. The path provided to the closure will be - /// inside of [`env::temp_dir()`]. Use [`Builder::make_in`] to provide - /// a custom temporary directory. If the closure returns one of the - /// following errors, then another randomized file path is tried: + /// provided closure. The closure is passed a [`MakeParams`], which + /// dereferences into the path at which the temporary file should be + /// created, and returns an [`std::io::Result`]. The path provided to the + /// closure will be inside of [`env::temp_dir()`]. Use [`Builder::make_in`] + /// to provide a custom temporary directory. If the closure returns one of + /// the following errors, then another randomized file path is tried: + /// /// - [`std::io::ErrorKind::AlreadyExists`] /// - [`std::io::ErrorKind::AddrInUse`] /// @@ -632,8 +635,6 @@ impl<'a> Builder<'a> { /// also enables creating a temporary UNIX domain socket, since it is not /// possible to bind to a socket that already exists. /// - /// Note that [`Builder::append`] is ignored when using [`Builder::make`]. - /// /// # Security /// /// This has the same [security implications][security] as @@ -649,8 +650,8 @@ impl<'a> Builder<'a> { /// use tempfile::Builder; /// /// // This is NOT secure! - /// let tempfile = Builder::new().make(|path| { - /// if path.is_file() { + /// let tempfile = Builder::new().make(|params| { + /// if params.is_file() { /// return Err(std::io::ErrorKind::AlreadyExists.into()); /// } /// @@ -658,7 +659,7 @@ impl<'a> Builder<'a> { /// // have replaced `path` with another file, which would get truncated /// // by `File::create`. /// - /// File::create(path) + /// File::create(params) /// })?; /// # Ok::<(), std::io::Error>(()) /// ``` @@ -695,7 +696,7 @@ impl<'a> Builder<'a> { /// use std::os::unix::net::UnixListener; /// use tempfile::Builder; /// - /// let tempsock = Builder::new().make(|path| UnixListener::bind(path))?; + /// let tempsock = Builder::new().make(|params| UnixListener::bind(params))?; /// # } /// # Ok::<(), std::io::Error>(()) /// ``` @@ -705,7 +706,7 @@ impl<'a> Builder<'a> { /// [resource-leaking]: struct.NamedTempFile.html#resource-leaking pub fn make(&self, f: F) -> io::Result> where - F: FnMut(&Path) -> io::Result, + F: FnMut(&MakeParams<'_>) -> io::Result, { self.make_in(env::temp_dir(), f) } @@ -722,13 +723,13 @@ impl<'a> Builder<'a> { /// use tempfile::Builder; /// use std::os::unix::net::UnixListener; /// - /// let tempsock = Builder::new().make_in("./", |path| UnixListener::bind(path))?; + /// let tempsock = Builder::new().make_in("./", |params| UnixListener::bind(params))?; /// # } /// # Ok::<(), std::io::Error>(()) /// ``` pub fn make_in(&self, dir: P, mut f: F) -> io::Result> where - F: FnMut(&Path) -> io::Result, + F: FnMut(&MakeParams<'_>) -> io::Result, P: AsRef, { util::create_helper( @@ -738,10 +739,44 @@ impl<'a> Builder<'a> { self.random_len, move |path| { Ok(NamedTempFile::from_parts( - f(&path)?, + f(&MakeParams { + path: &path, + permissions: self.permissions.as_ref(), + append_only: self.append, + })?, TempPath::new(path, self.keep), )) }, ) } } + +/// Parameters passed to the callback in [`Builder::make`] and [`Builder::make_in`]. +#[derive(Debug, Clone, PartialEq)] +#[non_exhaustive] +pub struct MakeParams<'a> { + /// The path that should be used for the new temporary file. + pub path: &'a Path, + /// The permissions that should be used when creating the temporary file, if + /// specified by the user. + pub permissions: Option<&'a Permissions>, + /// Whether or not the new file should be opened in append-only mode. + pub append_only: bool, +} + +impl Deref for MakeParams<'_> { + type Target = Path; + + fn deref(&self) -> &Self::Target { + self.path + } +} + +impl AsRef for MakeParams<'_> +where + Path: AsRef, +{ + fn as_ref(&self) -> &T { + self.path.as_ref() + } +} From 7db589b0ba267cbd49325837777211d78e592838 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 19 Nov 2024 11:57:30 -0800 Subject: [PATCH 08/11] feat: update env --- src/env.rs | 68 +++++++++++++++++++++++++++++------------ src/file/imp/unix.rs | 2 +- src/file/imp/windows.rs | 2 +- src/lib.rs | 9 +++--- 4 files changed, 55 insertions(+), 26 deletions(-) diff --git a/src/env.rs b/src/env.rs index b95745105..b6a27161a 100644 --- a/src/env.rs +++ b/src/env.rs @@ -1,30 +1,34 @@ use std::env; +use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; // Once rust 1.70 is wide-spread (Debian stable), we can use OnceLock from stdlib. +use once_cell::sync::Lazy; use once_cell::sync::OnceCell as OnceLock; +#[cfg(doc)] +use crate::Builder; + +static ENV_TEMPDIR: Lazy = Lazy::new(env::temp_dir); static DEFAULT_TEMPDIR: OnceLock = OnceLock::new(); +static DEFAULT_PREFIX: OnceLock = OnceLock::new(); /// Override the default temporary directory (defaults to [`std::env::temp_dir`]). This function /// changes the _global_ default temporary directory for the entire program and should not be called /// except in exceptional cases where it's not configured correctly by the platform. Applications /// should first check if the path returned by [`env::temp_dir`] is acceptable. /// -/// Only the first call to this function will succeed. All further calls will fail with `Err(path)` -/// where `path` is previously set default temporary directory override. +/// Only the **first** call to this function will succeed and return `Ok(path)` where `path` is a +/// static reference to the temporary directory. All further calls will fail with `Err(path)` where +/// `path` is the previously set default temporary directory override. /// /// **NOTE:** This function does not check if the specified directory exists and/or is writable. -pub fn override_temp_dir(path: &Path) -> Result<(), PathBuf> { - let mut we_set = false; - let val = DEFAULT_TEMPDIR.get_or_init(|| { - we_set = true; - path.to_path_buf() - }); - if we_set { - Ok(()) - } else { - Err(val.to_owned()) +pub fn override_temp_dir(path: impl Into) -> Result<&'static Path, &'static Path> { + let mut path = Some(path.into()); + let val = DEFAULT_TEMPDIR.get_or_init(|| path.take().unwrap()); + match path { + Some(_) => Err(val), + None => Ok(val), } } @@ -34,11 +38,37 @@ pub fn override_temp_dir(path: &Path) -> Result<(), PathBuf> { /// This function simply delegates to [`std::env::temp_dir`] unless the default temporary directory /// has been override by a call to [`override_temp_dir`]. /// -/// **NOTE:** This function does check if the returned directory exists and/or is writable. -pub fn temp_dir() -> PathBuf { - DEFAULT_TEMPDIR - .get() - .map(|p| p.to_owned()) - // Don't cache this in case the user uses std::env::set to change the temporary directory. - .unwrap_or_else(env::temp_dir) +/// **NOTE:** +/// +/// 1. This function does check if the returned directory exists and/or is writable. +/// 2. This function caches the result of [`std::env::temp_dir`]. Any future changes to, e.g., the +/// `TMPDIR` environment variable won't have any effect. +pub fn temp_dir() -> &'static Path { + DEFAULT_TEMPDIR.get().unwrap_or_else(|| &ENV_TEMPDIR) +} + +/// Override the default prefix for new temporary files (defaults to "tmp"). This function changes +/// the _global_ default prefix used by the entire program and should only be used by the top-level +/// application. It's recommended that the top-level application call this function to specify an +/// application-specific prefix to make it easier to identify temporary files belonging to the +/// application. +/// +/// Only the **first** call to this function will succeed and return `Ok(prefix)` where `prefix` is +/// a static reference to the default temporary file prefix. All further calls will fail with +/// `Err(prefix)` where `prefix` is the previously set default temporary file prefix. +pub fn override_default_prefix( + prefix: impl Into, +) -> Result<&'static OsStr, &'static OsStr> { + let mut prefix = Some(prefix.into()); + let val = DEFAULT_PREFIX.get_or_init(|| prefix.take().unwrap()); + match prefix { + Some(_) => Err(val), + None => Ok(val), + } +} + +/// Returns the default prefix used for new temporary files if no prefix is explicitly specified via +/// [`Builder::prefix`]. +pub fn default_prefix() -> &'static OsStr { + DEFAULT_PREFIX.get().map(|p| &**p).unwrap_or("tmp".as_ref()) } diff --git a/src/file/imp/unix.rs b/src/file/imp/unix.rs index 0d0520b6e..d90b9072b 100644 --- a/src/file/imp/unix.rs +++ b/src/file/imp/unix.rs @@ -72,7 +72,7 @@ pub fn create(dir: &Path) -> io::Result { fn create_unix(dir: &Path) -> io::Result { util::create_helper( dir, - OsStr::new(".tmp"), + crate::env::default_prefix(), OsStr::new(""), crate::NUM_RAND_CHARS, |path| create_unlinked(&path), diff --git a/src/file/imp/windows.rs b/src/file/imp/windows.rs index 620b16e74..ba8c8e0d1 100644 --- a/src/file/imp/windows.rs +++ b/src/file/imp/windows.rs @@ -63,7 +63,7 @@ pub fn create_named( pub fn create(dir: &Path) -> io::Result { util::create_helper( dir, - OsStr::new(".tmp"), + crate::env::default_prefix(), OsStr::new(""), crate::NUM_RAND_CHARS, |path| { diff --git a/src/lib.rs b/src/lib.rs index b33b439d6..1accfd680 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -197,7 +197,6 @@ doc_comment::doctest!("../README.md"); const NUM_RETRIES: u32 = 65536; const NUM_RAND_CHARS: usize = 6; -const DEFAULT_PREFIX: &str = "tmp"; const DEFAULT_SUFFIX: &str = ""; use std::ffi::OsStr; @@ -460,7 +459,7 @@ impl<'a> Builder<'a> { /// # } /// # Ok::<(), std::io::Error>(()) /// ``` - pub const fn permissions(&mut self, permissions: std::fs::Permissions) -> &mut Self { + pub const fn permissions(&mut self, permissions: Permissions) -> &mut Self { self.permissions = Some(permissions); self } @@ -546,7 +545,7 @@ impl<'a> Builder<'a> { pub fn tempfile_in>(&self, dir: P) -> io::Result { util::create_helper( dir.as_ref(), - self.prefix.unwrap_or(DEFAULT_PREFIX.as_ref()), + self.prefix.unwrap_or(env::default_prefix()), self.suffix.unwrap_or(DEFAULT_SUFFIX.as_ref()), self.random_len, |path| { @@ -612,7 +611,7 @@ impl<'a> Builder<'a> { pub fn tempdir_in>(&self, dir: P) -> io::Result { util::create_helper( dir.as_ref(), - self.prefix.unwrap_or(DEFAULT_PREFIX.as_ref()), + self.prefix.unwrap_or(env::default_prefix()), self.suffix.unwrap_or(DEFAULT_SUFFIX.as_ref()), self.random_len, |path| dir::create(path, self.permissions.as_ref(), self.keep), @@ -734,7 +733,7 @@ impl<'a> Builder<'a> { { util::create_helper( dir.as_ref(), - self.prefix.unwrap_or(DEFAULT_PREFIX.as_ref()), + self.prefix.unwrap_or(env::default_prefix()), self.suffix.unwrap_or(DEFAULT_SUFFIX.as_ref()), self.random_len, move |path| { From cc92f445eb93aa8d094644528c012f0a4190b4a6 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 30 Jan 2025 13:10:03 -0800 Subject: [PATCH 09/11] feat: drop once_cell dependency --- Cargo.toml | 2 -- src/env.rs | 7 ++----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f76065ce4..cd8051782 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,8 +18,6 @@ description = "A library for managing temporary files and directories." [dependencies] fastrand = "2.1.1" -# Not available in stdlib until 1.70, but we support 1.63 to support Debian stable. -once_cell = { version = "1.19.0", default-features = false, features = ["std"] } [target.'cfg(any(unix, windows, target_os = "wasi"))'.dependencies] getrandom = { version = "0.3.0", default-features = false, optional = true } diff --git a/src/env.rs b/src/env.rs index b6a27161a..9ec18d940 100644 --- a/src/env.rs +++ b/src/env.rs @@ -1,15 +1,12 @@ use std::env; use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; - -// Once rust 1.70 is wide-spread (Debian stable), we can use OnceLock from stdlib. -use once_cell::sync::Lazy; -use once_cell::sync::OnceCell as OnceLock; +use std::sync::{LazyLock, OnceLock}; #[cfg(doc)] use crate::Builder; -static ENV_TEMPDIR: Lazy = Lazy::new(env::temp_dir); +static ENV_TEMPDIR: LazyLock = LazyLock::new(env::temp_dir); static DEFAULT_TEMPDIR: OnceLock = OnceLock::new(); static DEFAULT_PREFIX: OnceLock = OnceLock::new(); From 21669d8a280f2f4b47f4072170c7e4bb6bec1118 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sat, 8 Feb 2025 15:20:02 -0800 Subject: [PATCH 10/11] feat: use new absolute path function --- src/util.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util.rs b/src/util.rs index cea17bb79..31124e537 100644 --- a/src/util.rs +++ b/src/util.rs @@ -39,8 +39,7 @@ pub fn create_helper( let mut base = base; // re-borrow to shrink lifetime let base_path_storage; // slot to store the absolute path, if necessary. if !base.is_absolute() { - let cur_dir = std::env::current_dir()?; - base_path_storage = cur_dir.join(base); + base_path_storage = std::path::absolute(base)?; base = &base_path_storage; } From ba0d77b7414a98dec7deeb1f21b1ef9720932484 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 16 Feb 2025 14:23:50 -0800 Subject: [PATCH 11/11] feat: make the builder const-friendly To do this, I had to switch from `AsRef` to `&str` for prefixes and suffixes, but a quick search indicates that this will break approximately nothing. I'm sure it'll break _someone's_ code, but there's really no good reason to support arbitrary os-strings here. The benefit is that projects can now construct project-wide temporary file builders, storing them in constants. --- src/dir/mod.rs | 23 ++++++++--------------- src/env.rs | 9 +++------ src/file/imp/unix.rs | 3 +-- src/file/imp/windows.rs | 3 +-- src/file/mod.rs | 22 ++++++++-------------- src/lib.rs | 13 ++++++------- src/util.rs | 8 ++++---- tests/namedtempfile.rs | 18 ++++++++++++++++++ 8 files changed, 49 insertions(+), 50 deletions(-) diff --git a/src/dir/mod.rs b/src/dir/mod.rs index 3e2a156a5..ebca9c5a2 100644 --- a/src/dir/mod.rs +++ b/src/dir/mod.rs @@ -8,7 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::ffi::OsStr; use std::fs::remove_dir_all; use std::mem; use std::path::{self, Path, PathBuf}; @@ -268,8 +267,8 @@ impl TempDir { /// assert!(tmp_name.starts_with("foo-")); /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn with_prefix>(prefix: S) -> io::Result { - Builder::new().prefix(&prefix).tempdir() + pub fn with_prefix(prefix: &str) -> io::Result { + Builder::new().prefix(prefix).tempdir() } /// Attempts to make a temporary directory with the specified suffix inside of @@ -293,8 +292,8 @@ impl TempDir { /// assert!(tmp_name.ends_with("-foo")); /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn with_suffix>(suffix: S) -> io::Result { - Builder::new().suffix(&suffix).tempdir() + pub fn with_suffix(suffix: &str) -> io::Result { + Builder::new().suffix(suffix).tempdir() } /// Attempts to make a temporary directory with the specified prefix inside /// the specified directory. The directory and everything inside it will be @@ -317,11 +316,8 @@ impl TempDir { /// assert!(tmp_name.ends_with("-foo")); /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn with_suffix_in, P: AsRef>( - suffix: S, - dir: P, - ) -> io::Result { - Builder::new().suffix(&suffix).tempdir_in(dir) + pub fn with_suffix_in>(suffix: &str, dir: P) -> io::Result { + Builder::new().suffix(suffix).tempdir_in(dir) } /// Attempts to make a temporary directory with the specified prefix inside @@ -345,11 +341,8 @@ impl TempDir { /// assert!(tmp_name.starts_with("foo-")); /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn with_prefix_in, P: AsRef>( - prefix: S, - dir: P, - ) -> io::Result { - Builder::new().prefix(&prefix).tempdir_in(dir) + pub fn with_prefix_in>(prefix: &str, dir: P) -> io::Result { + Builder::new().prefix(prefix).tempdir_in(dir) } /// Accesses the [`Path`] to the temporary directory. diff --git a/src/env.rs b/src/env.rs index 9ec18d940..79ccedeec 100644 --- a/src/env.rs +++ b/src/env.rs @@ -1,5 +1,4 @@ use std::env; -use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; use std::sync::{LazyLock, OnceLock}; @@ -8,7 +7,7 @@ use crate::Builder; static ENV_TEMPDIR: LazyLock = LazyLock::new(env::temp_dir); static DEFAULT_TEMPDIR: OnceLock = OnceLock::new(); -static DEFAULT_PREFIX: OnceLock = OnceLock::new(); +static DEFAULT_PREFIX: OnceLock = OnceLock::new(); /// Override the default temporary directory (defaults to [`std::env::temp_dir`]). This function /// changes the _global_ default temporary directory for the entire program and should not be called @@ -53,9 +52,7 @@ pub fn temp_dir() -> &'static Path { /// Only the **first** call to this function will succeed and return `Ok(prefix)` where `prefix` is /// a static reference to the default temporary file prefix. All further calls will fail with /// `Err(prefix)` where `prefix` is the previously set default temporary file prefix. -pub fn override_default_prefix( - prefix: impl Into, -) -> Result<&'static OsStr, &'static OsStr> { +pub fn override_default_prefix(prefix: impl Into) -> Result<&'static str, &'static str> { let mut prefix = Some(prefix.into()); let val = DEFAULT_PREFIX.get_or_init(|| prefix.take().unwrap()); match prefix { @@ -66,6 +63,6 @@ pub fn override_default_prefix( /// Returns the default prefix used for new temporary files if no prefix is explicitly specified via /// [`Builder::prefix`]. -pub fn default_prefix() -> &'static OsStr { +pub fn default_prefix() -> &'static str { DEFAULT_PREFIX.get().map(|p| &**p).unwrap_or("tmp".as_ref()) } diff --git a/src/file/imp/unix.rs b/src/file/imp/unix.rs index d90b9072b..c99cc7653 100644 --- a/src/file/imp/unix.rs +++ b/src/file/imp/unix.rs @@ -1,4 +1,3 @@ -use std::ffi::OsStr; use std::fs::{self, File, OpenOptions}; use std::io; @@ -73,7 +72,7 @@ fn create_unix(dir: &Path) -> io::Result { util::create_helper( dir, crate::env::default_prefix(), - OsStr::new(""), + "", crate::NUM_RAND_CHARS, |path| create_unlinked(&path), ) diff --git a/src/file/imp/windows.rs b/src/file/imp/windows.rs index ba8c8e0d1..ae19bf438 100644 --- a/src/file/imp/windows.rs +++ b/src/file/imp/windows.rs @@ -1,4 +1,3 @@ -use std::ffi::OsStr; use std::fs::{File, OpenOptions}; use std::os::windows::ffi::OsStrExt; use std::os::windows::fs::OpenOptionsExt; @@ -64,7 +63,7 @@ pub fn create(dir: &Path) -> io::Result { util::create_helper( dir, crate::env::default_prefix(), - OsStr::new(""), + "", crate::NUM_RAND_CHARS, |path| { let f = OpenOptions::new() diff --git a/src/file/mod.rs b/src/file/mod.rs index bd50b5298..1bbe3d17b 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -567,8 +567,8 @@ impl NamedTempFile { /// See [`NamedTempFile::new()`] for details. /// /// [`NamedTempFile::new()`]: #method.new - pub fn with_suffix>(suffix: S) -> io::Result { - Builder::new().suffix(&suffix).tempfile() + pub fn with_suffix(suffix: &str) -> io::Result { + Builder::new().suffix(suffix).tempfile() } /// Create a new named temporary file with the specified filename suffix, /// in the specified directory. @@ -582,11 +582,8 @@ impl NamedTempFile { /// See [`NamedTempFile::new()`] for details. /// /// [`NamedTempFile::new()`]: #method.new - pub fn with_suffix_in, P: AsRef>( - suffix: S, - dir: P, - ) -> io::Result { - Builder::new().suffix(&suffix).tempfile_in(dir) + pub fn with_suffix_in>(suffix: &str, dir: P) -> io::Result { + Builder::new().suffix(suffix).tempfile_in(dir) } /// Create a new named temporary file with the specified filename prefix. @@ -594,8 +591,8 @@ impl NamedTempFile { /// See [`NamedTempFile::new()`] for details. /// /// [`NamedTempFile::new()`]: #method.new - pub fn with_prefix>(prefix: S) -> io::Result { - Builder::new().prefix(&prefix).tempfile() + pub fn with_prefix(prefix: &str) -> io::Result { + Builder::new().prefix(prefix).tempfile() } /// Create a new named temporary file with the specified filename prefix, /// in the specified directory. @@ -609,11 +606,8 @@ impl NamedTempFile { /// See [`NamedTempFile::new()`] for details. /// /// [`NamedTempFile::new()`]: #method.new - pub fn with_prefix_in, P: AsRef>( - prefix: S, - dir: P, - ) -> io::Result { - Builder::new().prefix(&prefix).tempfile_in(dir) + pub fn with_prefix_in>(prefix: &str, dir: P) -> io::Result { + Builder::new().prefix(prefix).tempfile_in(dir) } } diff --git a/src/lib.rs b/src/lib.rs index 1accfd680..cbdb246e0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -199,7 +199,6 @@ const NUM_RETRIES: u32 = 65536; const NUM_RAND_CHARS: usize = 6; const DEFAULT_SUFFIX: &str = ""; -use std::ffi::OsStr; use std::fs::{OpenOptions, Permissions}; use std::io; use std::ops::Deref; @@ -223,8 +222,8 @@ pub use crate::spooled::{spooled_tempfile, SpooledData, SpooledTempFile}; #[derive(Debug, Clone, Eq, PartialEq)] pub struct Builder<'a> { random_len: usize, - prefix: Option<&'a OsStr>, - suffix: Option<&'a OsStr>, + prefix: Option<&'a str>, + suffix: Option<&'a str>, append: bool, permissions: Option, keep: bool, @@ -329,8 +328,8 @@ impl<'a> Builder<'a> { /// .tempfile()?; /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn prefix + ?Sized>(&mut self, prefix: &'a S) -> &mut Self { - self.prefix = Some(prefix.as_ref()); + pub const fn prefix(&mut self, prefix: &'a str) -> &mut Self { + self.prefix = Some(prefix); self } @@ -349,8 +348,8 @@ impl<'a> Builder<'a> { /// .tempfile()?; /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn suffix + ?Sized>(&mut self, suffix: &'a S) -> &mut Self { - self.suffix = Some(suffix.as_ref()); + pub const fn suffix(&mut self, suffix: &'a str) -> &mut Self { + self.suffix = Some(suffix); self } diff --git a/src/util.rs b/src/util.rs index 31124e537..ee57a8a74 100644 --- a/src/util.rs +++ b/src/util.rs @@ -6,8 +6,8 @@ use crate::error::IoResultExt; fn tmpname( rng: &mut fastrand::Rng, - prefix: &OsStr, - suffix: &OsStr, + prefix: &str, + suffix: &str, rand_len: usize, ) -> io::Result { let capacity = prefix @@ -29,8 +29,8 @@ fn tmpname( pub fn create_helper( base: &Path, - prefix: &OsStr, - suffix: &OsStr, + prefix: &str, + suffix: &str, random_len: usize, mut f: impl FnMut(PathBuf) -> io::Result, ) -> io::Result { diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs index 88522266a..0ab957e78 100644 --- a/tests/namedtempfile.rs +++ b/tests/namedtempfile.rs @@ -24,6 +24,24 @@ fn test_suffix() { assert!(name.ends_with("suffix")); } +static TEST_BUILDER: Builder<'static> = { + let mut b = Builder::new(); + b.prefix("prefix"); + b +}; + +#[test] +fn test_static_builder() { + let tmpfile = TEST_BUILDER + .clone() + .suffix(&String::from("suffix")) + .tempfile() + .unwrap(); + let name = tmpfile.path().file_name().unwrap().to_str().unwrap(); + assert!(name.starts_with("prefix")); + assert!(name.ends_with("suffix")); +} + #[test] fn test_basic() { let mut tmpfile = NamedTempFile::new().unwrap();