CHANGELOG.md

@@ -1,5 +1,22 @@
 # Changelog
 
+## 3.18.0
+
+- Update `rustix` to 1.0.0.
+- Make `NamedTempFile::persist_noclobber` atomic on Apple operating systems. It's now atomic on MacOS, Windows, and Linux (depending on the OS version and filesystem used).
+
+## 3.17.1
+
+- Fix build with `windows-sys` 0.52. Unfortunately, we have no CI for older `windows-sys` versions at the moment...
+
+## 3.17.0
+
+- Make sure to use absolute paths in when creating unnamed temporary files (avoids a small race in the "immediate unlink" logic) and in `Builder::make_in` (when creating temporary files of arbitrary types).
+- Prevent a theoretical crash that could (maybe) happen when a temporary file is created from a drop function run in a TLS destructor. Nobody has actually reported a case of this happening in practice and I have been unable to create this scenario in a test.
+- When reseeding with `getrandom`, use platform (e.g., CPU) specific randomness sources where possible.
+- Clarify some documentation.
+- Unlink unnamed temporary files on windows _immediately_ when possible instead of waiting for the handle to be closed. We open files with "Unix" semantics, so this is generally possible.
+
 ## 3.16.0
 
 - Update `getrandom` to `0.3.0` (thanks to @paolobarbolini).

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "tempfile"
-version = "3.16.0"
+version = "3.18.0"
 authors = [
   "Steven Allen <steven@stebalien.com>",
   "The Rust Project Developers",
@@ -17,14 +17,13 @@ repository = "https://github.com/Stebalien/tempfile"
 description = "A library for managing temporary files and directories."
 
 [dependencies]
-cfg-if = "1"
 fastrand = "2.1.1"
 
 [target.'cfg(any(unix, windows, target_os = "wasi"))'.dependencies]
 getrandom = { version = "0.3.0", default-features = false, optional = true }
 
 [target.'cfg(any(unix, target_os = "wasi"))'.dependencies]
-rustix = { version = "0.38.39", features = ["fs"] }
+rustix = { version = "1.0.0", features = ["fs"] }
 
 [target.'cfg(windows)'.dependencies.windows-sys]
 version = ">=0.52,<0.60"

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<S: AsRef<OsStr>>(prefix: S) -> io::Result<TempDir> {
-        Builder::new().prefix(&prefix).tempdir()
+    pub fn with_prefix(prefix: &str) -> io::Result<TempDir> {
+        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<S: AsRef<OsStr>>(suffix: S) -> io::Result<TempDir> {
-        Builder::new().suffix(&suffix).tempdir()
+    pub fn with_suffix(suffix: &str) -> io::Result<TempDir> {
+        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<S: AsRef<OsStr>, P: AsRef<Path>>(
-        suffix: S,
-        dir: P,
-    ) -> io::Result<TempDir> {
-        Builder::new().suffix(&suffix).tempdir_in(dir)
+    pub fn with_suffix_in<P: AsRef<Path>>(suffix: &str, dir: P) -> io::Result<TempDir> {
+        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<S: AsRef<OsStr>, P: AsRef<Path>>(
-        prefix: S,
-        dir: P,
-    ) -> io::Result<TempDir> {
-        Builder::new().prefix(&prefix).tempdir_in(dir)
+    pub fn with_prefix_in<P: AsRef<Path>>(prefix: &str, dir: P) -> io::Result<TempDir> {
+        Builder::new().prefix(prefix).tempdir_in(dir)
     }
 
     /// Accesses the [`Path`] to the temporary directory.

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<PathBuf> = LazyLock::new(env::temp_dir);
 static DEFAULT_TEMPDIR: OnceLock<PathBuf> = OnceLock::new();
-static DEFAULT_PREFIX: OnceLock<OsString> = OnceLock::new();
+static DEFAULT_PREFIX: OnceLock<String> = 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<OsString>,
-) -> Result<&'static OsStr, &'static OsStr> {
+pub fn override_default_prefix(prefix: impl Into<String>) -> 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())
 }

src/file/imp/mod.rs

@@ -1,12 +1,9 @@
-cfg_if::cfg_if! {
-    if #[cfg(any(unix, target_os = "redox", target_os = "wasi"))] {
-        mod unix;
-        pub use self::unix::*;
-    } else if #[cfg(windows)] {
-        mod windows;
-        pub use self::windows::*;
-    } else {
-        mod other;
-        pub use self::other::*;
-    }
-}
+#[cfg_attr(any(unix, target_os = "redox", target_os = "wasi"), path = "unix.rs")]
+#[cfg_attr(windows, path = "windows.rs")]
+#[cfg_attr(
+    not(any(unix, target_os = "redox", target_os = "wasi", windows)),
+    path = "other.rs"
+)]
+mod platform;
+
+pub use self::platform::*;

src/file/imp/unix.rs

@@ -1,14 +1,6 @@
-use std::ffi::OsStr;
 use std::fs::{self, File, OpenOptions};
 use std::io;
-cfg_if::cfg_if! {
-    if #[cfg(not(target_os = "wasi"))] {
-        use std::os::unix::fs::MetadataExt;
-    } else {
-        #[cfg(feature = "nightly")]
-        use std::os::wasi::fs::MetadataExt;
-    }
-}
+
 use crate::util;
 use std::path::Path;
 
@@ -80,14 +72,19 @@ fn create_unix(dir: &Path) -> io::Result<File> {
     util::create_helper(
         dir,
         crate::env::default_prefix(),
-        OsStr::new(""),
+        "",
         crate::NUM_RAND_CHARS,
         |path| create_unlinked(&path),
     )
 }
 
 #[cfg(any(not(target_os = "wasi"), feature = "nightly"))]
 pub fn reopen(file: &File, path: &Path) -> io::Result<File> {
+    #[cfg(not(target_os = "wasi"))]
+    use std::os::unix::fs::MetadataExt;
+    #[cfg(target_os = "wasi")]
+    use std::os::wasi::fs::MetadataExt;
+
     let new_file = OpenOptions::new().read(true).write(true).open(path)?;
     let old_meta = file.metadata()?;
     let new_meta = new_file.metadata()?;
@@ -113,9 +110,17 @@ pub fn persist(old_path: &Path, new_path: &Path, overwrite: bool) -> io::Result<
     if overwrite {
         rename(old_path, new_path)?;
     } else {
-        // On Linux, use `renameat_with` to avoid overwriting an existing name,
-        // if the kernel and the filesystem support it.
-        #[cfg(any(target_os = "android", target_os = "linux"))]
+        // On Linux and apple operating systems, use `renameat_with` to avoid overwriting an
+        // existing name, if the kernel and the filesystem support it.
+        #[cfg(any(
+            target_os = "android",
+            target_os = "linux",
+            target_os = "macos",
+            target_os = "ios",
+            target_os = "tvos",
+            target_os = "visionos",
+            target_os = "watchos",
+        ))]
         {
             use rustix::fs::{renameat_with, RenameFlags, CWD};
             use rustix::io::Errno;

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;
@@ -31,7 +30,7 @@ fn delete_open_file(f: &File) -> io::Result<()> {
             Flags: FILE_DISPOSITION_FLAG_DELETE | FILE_DISPOSITION_FLAG_POSIX_SEMANTICS,
         };
         if SetFileInformationByHandle(
-            f.as_raw_handle(),
+            f.as_raw_handle() as HANDLE,
             FileDispositionInfoEx,
             &info as *const _ as *const _,
             std::mem::size_of::<FILE_DISPOSITION_INFO_EX>() as u32,
@@ -64,7 +63,7 @@ pub fn create(dir: &Path) -> io::Result<File> {
     util::create_helper(
         dir,
         crate::env::default_prefix(),
-        OsStr::new(""),
+        "",
         crate::NUM_RAND_CHARS,
         |path| {
             let f = OpenOptions::new()

src/file/mod.rs

@@ -567,8 +567,8 @@ impl NamedTempFile<File> {
     /// See [`NamedTempFile::new()`] for details.
     ///
     /// [`NamedTempFile::new()`]: #method.new
-    pub fn with_suffix<S: AsRef<OsStr>>(suffix: S) -> io::Result<NamedTempFile> {
-        Builder::new().suffix(&suffix).tempfile()
+    pub fn with_suffix(suffix: &str) -> io::Result<NamedTempFile> {
+        Builder::new().suffix(suffix).tempfile()
     }
     /// Create a new named temporary file with the specified filename suffix,
     /// in the specified directory.
@@ -582,20 +582,17 @@ impl NamedTempFile<File> {
     /// See [`NamedTempFile::new()`] for details.
     ///
     /// [`NamedTempFile::new()`]: #method.new
-    pub fn with_suffix_in<S: AsRef<OsStr>, P: AsRef<Path>>(
-        suffix: S,
-        dir: P,
-    ) -> io::Result<NamedTempFile> {
-        Builder::new().suffix(&suffix).tempfile_in(dir)
+    pub fn with_suffix_in<P: AsRef<Path>>(suffix: &str, dir: P) -> io::Result<NamedTempFile> {
+        Builder::new().suffix(suffix).tempfile_in(dir)
     }
 
     /// Create a new named temporary file with the specified filename prefix.
     ///
     /// See [`NamedTempFile::new()`] for details.
     ///
     /// [`NamedTempFile::new()`]: #method.new
-    pub fn with_prefix<S: AsRef<OsStr>>(prefix: S) -> io::Result<NamedTempFile> {
-        Builder::new().prefix(&prefix).tempfile()
+    pub fn with_prefix(prefix: &str) -> io::Result<NamedTempFile> {
+        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<File> {
     /// See [`NamedTempFile::new()`] for details.
     ///
     /// [`NamedTempFile::new()`]: #method.new
-    pub fn with_prefix_in<S: AsRef<OsStr>, P: AsRef<Path>>(
-        prefix: S,
-        dir: P,
-    ) -> io::Result<NamedTempFile> {
-        Builder::new().prefix(&prefix).tempfile_in(dir)
+    pub fn with_prefix_in<P: AsRef<Path>>(prefix: &str, dir: P) -> io::Result<NamedTempFile> {
+        Builder::new().prefix(prefix).tempfile_in(dir)
     }
 }
 
@@ -675,7 +669,7 @@ impl<F> NamedTempFile<F> {
     /// If this method fails, it will return `self` in the resulting
     /// [`PersistError`].
     ///
-    /// Note: Temporary files cannot be persisted across filesystems. Also
+    /// **Note:** Temporary files cannot be persisted across filesystems. Also
     /// neither the file contents nor the containing directory are
     /// synchronized, so the update may not yet have reached the disk when
     /// `persist` returns.
@@ -723,9 +717,15 @@ impl<F> NamedTempFile<F> {
     /// If a file exists at the target path, fail. If this method fails, it will
     /// return `self` in the resulting PersistError.
     ///
-    /// Note: Temporary files cannot be persisted across filesystems. Also Note:
-    /// This method is not atomic. It can leave the original link to the
-    /// temporary file behind.
+    /// **Note:** Temporary files cannot be persisted across filesystems.
+    ///
+    /// **Atomicity:** This method is not guaranteed to be atomic on all platforms, although it will
+    /// generally be atomic on Windows and modern Linux filesystems. While it will never overwrite a
+    /// file at the target path, it may leave the original link to the temporary file behind leaving
+    /// you with two [hard links][hardlink] in your filesystem pointing at the same underlying file.
+    /// This can happen if either (a) we lack permission to "unlink" the original filename; (b) this
+    /// program crashes while persisting the temporary file; or (c) the filesystem is removed,
+    /// unmounted, etc. while we're performing this operation.
     ///
     /// # Security
     ///
@@ -750,6 +750,8 @@ impl<F> NamedTempFile<F> {
     /// writeln!(persisted_file, "Brian was here. Briefly.")?;
     /// # Ok::<(), std::io::Error>(())
     /// ```
+    ///
+    /// [hardlink]: https://en.wikipedia.org/wiki/Hard_link
     pub fn persist_noclobber<P: AsRef<Path>>(self, new_path: P) -> Result<F, PersistError<F>> {
         let NamedTempFile { path, file } = self;
         match path.persist_noclobber(new_path) {

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<Permissions>,
     keep: bool,
@@ -329,8 +328,8 @@ impl<'a> Builder<'a> {
     ///     .tempfile()?;
     /// # Ok::<(), std::io::Error>(())
     /// ```
-    pub fn prefix<S: AsRef<OsStr> + ?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<S: AsRef<OsStr> + ?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
     }
 

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<OsString> {
     let capacity = prefix
@@ -29,8 +29,8 @@ fn tmpname(
 
 pub fn create_helper<R>(
     base: &Path,
-    prefix: &OsStr,
-    suffix: &OsStr,
+    prefix: &str,
+    suffix: &str,
     random_len: usize,
     mut f: impl FnMut(PathBuf) -> io::Result<R>,
 ) -> io::Result<R> {