From 1bda38185cf2e4d896f7c6638ba130db8def2a11 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sat, 15 Feb 2025 15:12:52 -0800 Subject: [PATCH 01/16] feat: update MSRV from 1.63 to 1.85 --- .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 d8f8cb521..9f3513794 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,7 @@ jobs: rust-version: - nightly - stable - - "1.63" + - "1.85" platform: - name: "Linux" os: ubuntu-latest diff --git a/Cargo.toml b/Cargo.toml index 5ab5c0a16..5e219e674 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.85" 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 b210a421a..d3a171c96 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ patterns and surprisingly difficult to implement securely). ## Usage -Minimum required Rust version: 1.63.0 +Minimum required Rust version: 1.85.0 Add this to your `Cargo.toml`: From 7de2281d493a2bbdad44321092b7639a15c3a5ce Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 13 Mar 2026 11:28:14 -0700 Subject: [PATCH 02/16] feat: switch to rust 2024 edition --- Cargo.toml | 2 +- src/dir/imp/any.rs | 2 +- src/dir/imp/unix.rs | 2 +- src/dir/mod.rs | 2 +- src/env.rs | 8 ++------ src/file/imp/unix.rs | 2 +- src/file/imp/windows.rs | 6 +++--- src/file/mod.rs | 2 +- src/lib.rs | 6 +++--- tests/namedtempfile.rs | 2 +- tests/spooled.rs | 2 +- tests/tempfile.rs | 2 +- 12 files changed, 17 insertions(+), 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5e219e674..4bde72a7c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ authors = [ "Jason White ", ] documentation = "https://docs.rs/tempfile" -edition = "2021" +edition = "2024" rust-version = "1.85" homepage = "https://stebalien.com/projects/tempfile-rs/" keywords = ["tempfile", "tmpfile", "filesystem"] diff --git a/src/dir/imp/any.rs b/src/dir/imp/any.rs index e938d87c2..184f39376 100644 --- a/src/dir/imp/any.rs +++ b/src/dir/imp/any.rs @@ -1,5 +1,5 @@ -use crate::error::IoResultExt; use crate::TempDir; +use crate::error::IoResultExt; use std::path::PathBuf; use std::{fs, io}; diff --git a/src/dir/imp/unix.rs b/src/dir/imp/unix.rs index 54f305e98..1cacacebe 100644 --- a/src/dir/imp/unix.rs +++ b/src/dir/imp/unix.rs @@ -1,5 +1,5 @@ -use crate::error::IoResultExt; use crate::TempDir; +use crate::error::IoResultExt; use std::io; use std::path::PathBuf; diff --git a/src/dir/mod.rs b/src/dir/mod.rs index 93a8d7ccb..1bafa3340 100644 --- a/src/dir/mod.rs +++ b/src/dir/mod.rs @@ -12,8 +12,8 @@ use std::mem; use std::path::{self, Path, PathBuf}; use std::{fmt, io}; -use crate::error::IoResultExt; use crate::Builder; +use crate::error::IoResultExt; #[cfg(doc)] use crate::env; diff --git a/src/env.rs b/src/env.rs index 4276b9ad4..5e70c4a01 100644 --- a/src/env.rs +++ b/src/env.rs @@ -2,7 +2,7 @@ use std::env; use std::path::{Path, PathBuf}; #[cfg(doc)] -use crate::{tempdir_in, tempfile_in, Builder}; +use crate::{Builder, tempdir_in, tempfile_in}; // Once rust 1.70 is wide-spread (Debian stable), we can use OnceLock from stdlib. use once_cell::sync::OnceCell as OnceLock; @@ -28,11 +28,7 @@ pub fn override_temp_dir(path: &Path) -> Result<(), PathBuf> { we_set = true; path.to_path_buf() }); - if we_set { - Ok(()) - } else { - Err(val.to_owned()) - } + if we_set { Ok(()) } else { Err(val.to_owned()) } } /// Returns the default temporary directory, used for both temporary directories and files if no diff --git a/src/file/imp/unix.rs b/src/file/imp/unix.rs index 987791e14..b7b63e127 100644 --- a/src/file/imp/unix.rs +++ b/src/file/imp/unix.rs @@ -108,7 +108,7 @@ pub fn persist(old_path: &Path, new_path: &Path, overwrite: bool) -> io::Result< target_os = "redox", ))] { - use rustix::fs::{renameat_with, RenameFlags, CWD}; + use rustix::fs::{CWD, RenameFlags, renameat_with}; use rustix::io::Errno; use std::sync::atomic::{AtomicBool, Ordering::Relaxed}; diff --git a/src/file/imp/windows.rs b/src/file/imp/windows.rs index 3ef149d6f..0c2bdf836 100644 --- a/src/file/imp/windows.rs +++ b/src/file/imp/windows.rs @@ -8,9 +8,9 @@ use std::{io, iter}; use windows_sys::Win32::Foundation::{HANDLE, INVALID_HANDLE_VALUE}; use windows_sys::Win32::Storage::FileSystem::{ - MoveFileExW, ReOpenFile, SetFileAttributesW, FILE_ATTRIBUTE_NORMAL, FILE_ATTRIBUTE_TEMPORARY, - FILE_FLAG_DELETE_ON_CLOSE, FILE_GENERIC_READ, FILE_GENERIC_WRITE, FILE_SHARE_DELETE, - FILE_SHARE_READ, FILE_SHARE_WRITE, MOVEFILE_REPLACE_EXISTING, + FILE_ATTRIBUTE_NORMAL, FILE_ATTRIBUTE_TEMPORARY, FILE_FLAG_DELETE_ON_CLOSE, FILE_GENERIC_READ, + FILE_GENERIC_WRITE, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, + MOVEFILE_REPLACE_EXISTING, MoveFileExW, ReOpenFile, SetFileAttributesW, }; use crate::util; diff --git a/src/file/mod.rs b/src/file/mod.rs index d2b5ec208..8cab4ad41 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -13,9 +13,9 @@ use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, RawFd}; use std::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, RawHandle}; use std::path::{Path, PathBuf}; +use crate::Builder; use crate::env; use crate::error::IoResultExt; -use crate::Builder; mod imp; diff --git a/src/lib.rs b/src/lib.rs index 5dff328a5..fb20db835 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -207,11 +207,11 @@ mod util; pub mod env; -pub use crate::dir::{tempdir, tempdir_in, TempDir}; +pub use crate::dir::{TempDir, tempdir, tempdir_in}; pub use crate::file::{ - tempfile, tempfile_in, NamedTempFile, PathPersistError, PersistError, TempPath, + NamedTempFile, PathPersistError, PersistError, TempPath, tempfile, tempfile_in, }; -pub use crate::spooled::{spooled_tempfile, spooled_tempfile_in, SpooledData, SpooledTempFile}; +pub use crate::spooled::{SpooledData, SpooledTempFile, spooled_tempfile, spooled_tempfile_in}; /// Create a new temporary file or directory with custom options. #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs index 88e31bccd..8648a95d7 100644 --- a/tests/namedtempfile.rs +++ b/tests/namedtempfile.rs @@ -4,7 +4,7 @@ use std::ffi::{OsStr, OsString}; use std::fs::File; use std::io::{self, Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; -use tempfile::{env, tempdir, Builder, NamedTempFile, TempPath}; +use tempfile::{Builder, NamedTempFile, TempPath, env, tempdir}; struct CWDGuard { #[allow(unused)] diff --git a/tests/spooled.rs b/tests/spooled.rs index 21cfeb7eb..50529d963 100644 --- a/tests/spooled.rs +++ b/tests/spooled.rs @@ -2,7 +2,7 @@ use std::io::{Read, Seek, SeekFrom, Write}; -use tempfile::{env, spooled_tempfile, spooled_tempfile_in, SpooledTempFile}; +use tempfile::{SpooledTempFile, env, spooled_tempfile, spooled_tempfile_in}; /// For the wasi platforms, `std::env::temp_dir` will panic. For those targets, configure the /tmp /// directory instead as the base directory for temp files. diff --git a/tests/tempfile.rs b/tests/tempfile.rs index 8d245c5fe..938a9eb24 100644 --- a/tests/tempfile.rs +++ b/tests/tempfile.rs @@ -4,7 +4,7 @@ use std::fs; use std::io::{Read, Seek, SeekFrom, Write}; #[cfg(target_os = "linux")] use std::{ - sync::mpsc::{sync_channel, TryRecvError}, + sync::mpsc::{TryRecvError, sync_channel}, thread, }; From 8bd4c1c2c19f759121492a014ca4f83d5ef82666 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 13 Mar 2026 12:13:40 -0700 Subject: [PATCH 03/16] ci: use the current compiler version to resolve dependencies I'm testing specific compiler versions, not the MSRV. This seems to be the easiest way to do this reliably. I've also changed the cargo config to by-default allow incompatible versions. That way, people running `cargo test` locally will get the latest dependencies instead of outdated dependencies. I expect anyone working on tempfile to use the latest compiler anyways. Also, auto-detect MSRV --- .cargo/config.toml | 2 ++ .github/workflows/ci.yml | 64 +++++++++++++++++++--------------------- 2 files changed, 33 insertions(+), 33 deletions(-) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000000000..04ee75f37 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,2 @@ +[resolver] +incompatible-rust-versions = "allow" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9f3513794..ba57a9241 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,11 @@ env: ANDROID_VERSION: "12.0.0" WASMTIME_BACKTRACE_DETAILS: 1 +# Because Windows on GitHub CI +defaults: + run: + shell: bash + permissions: {} name: CI @@ -33,14 +38,32 @@ jobs: - uses: EmbarkStudios/cargo-deny-action@91bf2b620e09e18d6eb78b92e7861937469acedb # v2 timeout-minutes: 10 + determine_msrv: + name: Determining MSRV + runs-on: ubuntu-latest + outputs: + msrv: ${{ steps.msrv.outputs.MSRV }} + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false + - name: Determine MSRV + id: msrv + run: | + cargo metadata --no-deps --format-version 1 \ + | jq -r '.packages[] | select(.name == "tempfile") | "MSRV=\(.rust_version)"' \ + >> "$GITHUB_OUTPUT" + build_and_test: + needs: determine_msrv strategy: fail-fast: false matrix: rust-version: - nightly - stable - - "1.85" + - "${{ needs.determine_msrv.outputs.msrv }}" platform: - name: "Linux" os: ubuntu-latest @@ -62,24 +85,6 @@ jobs: os: ubuntu-latest target: wasm32-wasip2 test-flags: --tests - exclude: - - rust-version: "1.63" - platform: - name: "Android" - os: ubuntu-latest - target: x86_64-linux-android - - rust-version: "1.63" - platform: - name: "WASI P1" - os: ubuntu-latest - target: wasm32-wasip1 - test-flags: --tests - - rust-version: "1.63" - platform: - name: "WASI P2" - os: ubuntu-latest - target: wasm32-wasip2 - test-flags: --tests name: Platform Test (${{ matrix.platform.name }} / ${{ matrix.rust-version }}) runs-on: ${{ matrix.platform.os }} steps: @@ -92,20 +97,13 @@ jobs: with: toolchain: ${{ matrix.rust-version }} targets: ${{ matrix.platform.target }} - - name: Generating the Cargo.lock - run: cargo generate-lockfile - - name: Downgrading once_cell - if: matrix.rust-version == 1.63 - run: cargo update -p once_cell --precise 1.20.3 - - name: Downgrading windows-sys - if: matrix.rust-version == 1.63 - run: cargo update -p windows-sys --precise 0.60.2 - - name: Downgrading getrandom - if: matrix.rust-version == 1.63 - run: cargo update -p getrandom --precise 0.3.4 - - name: Downgrading libc - if: matrix.rust-version == 1.63 - run: cargo update -p libc --precise 0.2.183 + - name: Selecting dependency versions + env: + CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS: 'fallback' + run: | + RUSTC_VERSION="$(rustc --version | sed 's/^rustc \([^ -]*\).*/\1/')" + sed -i -e 's/^rust-version = .*/rust-version = "'"$RUSTC_VERSION"'"/' Cargo.toml + cargo generate-lockfile - name: Install Wasmtime if: ${{ startsWith(matrix.platform.target, 'wasm32-wasi') }} run: curl https://wasmtime.dev/install.sh -sSf | bash From d44a74864a4e4a274b9d3ed732b4cdf276f96d4f Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 13 Mar 2026 11:33:04 -0700 Subject: [PATCH 04/16] chore: fix clippy lints --- tests/namedtempfile.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs index 8648a95d7..aed1c22b3 100644 --- a/tests/namedtempfile.rs +++ b/tests/namedtempfile.rs @@ -637,7 +637,7 @@ fn test_reseed() { let mut files: Vec<_> = Vec::new(); let _ = Builder::new().make(|path| -> io::Result { if attempts == 5 { - return Err(io::Error::new(io::ErrorKind::Other, "stop!")); + return Err(io::Error::other("stop!")); } attempts += 1; let f = File::options() From 1e6a19ee7c3519dd80f96fdef51a30aa04fdf368 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sat, 10 May 2025 13:29:49 -0400 Subject: [PATCH 05/16] feat: remove deprecated methods --- Cargo.toml | 1 - src/dir/mod.rs | 7 ------- src/file/mod.rs | 29 ----------------------------- src/lib.rs | 6 ------ tests/namedtempfile.rs | 24 ------------------------ 5 files changed, 67 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4bde72a7c..40ab7b239 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,7 +41,6 @@ doc-comment = "0.3" [features] default = ["getrandom"] -nightly = [] # DEPRECATED [package.metadata.docs.rs] rustdoc-args = ["--generate-link-to-definition"] diff --git a/src/dir/mod.rs b/src/dir/mod.rs index 1bafa3340..84815e80a 100644 --- a/src/dir/mod.rs +++ b/src/dir/mod.rs @@ -380,13 +380,6 @@ impl TempDir { self.path.as_ref() } - /// Deprecated alias for [`TempDir::keep`]. - #[must_use] - #[deprecated = "use TempDir::keep()"] - pub fn into_path(self) -> PathBuf { - self.keep() - } - /// Persist the temporary directory to disk, returning the [`PathBuf`] where it is located. /// /// This consumes the [`TempDir`] without deleting directory on the filesystem, meaning that diff --git a/src/file/mod.rs b/src/file/mod.rs index 8cab4ad41..01aabb4fe 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -325,35 +325,6 @@ impl TempPath { self.disable_cleanup = disable_cleanup } - /// Create a new TempPath from an existing path. This can be done even if no file exists at the - /// given path. - /// - /// This is mostly useful for interacting with libraries and external components that provide - /// files to be consumed or expect a path with no existing file to be given. - /// - /// When passed a relative path, this function first tries to make it absolute (relative to the - /// current directory). If this fails, this function uses the relative path as-is. - /// - /// **DEPRECATED:** Use [`TempPath::try_from_path`] instead to handle the case where looking up - /// the current directory fails. - #[deprecated = "use TempPath::try_from_path"] - pub fn from_path(path: impl Into) -> Self { - let mut path = path.into(); - // Best effort to resolve a relative path. If we fail, we keep the path as-is to - // preserve backwards compatibility. - // - // Ignore empty paths entirely. There's nothing we can do about them. - if path != Path::new("") && !path.is_absolute() { - if let Ok(cur_dir) = std::env::current_dir() { - path = cur_dir.join(path); - } - } - Self { - path: path.into_boxed_path(), - disable_cleanup: false, - } - } - /// Create a new TempPath from an existing path. This can be done even if no file exists at the /// given path. /// diff --git a/src/lib.rs b/src/lib.rs index fb20db835..3271ac622 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -492,12 +492,6 @@ impl<'a, 'b> Builder<'a, 'b> { self } - /// Deprecated alias for [`Builder::disable_cleanup`]. - #[deprecated = "Use Builder::disable_cleanup"] - pub fn keep(&mut self, keep: bool) -> &mut Self { - self.disable_cleanup(keep) - } - /// Create the named temporary file. /// /// # Security diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs index aed1c22b3..8126ff627 100644 --- a/tests/namedtempfile.rs +++ b/tests/namedtempfile.rs @@ -348,36 +348,12 @@ fn test_temp_path_resolve_missing_cwd() { std::env::set_current_dir(&tmpdir).expect("failed to change to the temporary directory"); tmpdir.close().unwrap(); - #[allow(deprecated)] - let path = TempPath::from_path("foo"); - assert_eq!(&*path, Path::new("foo")); - TempPath::try_from_path("foo").expect_err("should have failed to make path absolute file"); } #[test] fn test_temp_path_resolve_existing_cwd() { configure_wasi_temp_dir(); - let _guard = cwd_lock(); - - let tmpdir = tempdir().unwrap(); - std::env::set_current_dir(&tmpdir).expect("failed to change to directory"); - - let cwd = if cfg!(target_os = "macos") { - // MacOS has absolute paths and ABSOLUTE paths. `cd /var/tmp/...` actually changes to - // /private/var/tmp... - std::env::current_dir().expect("failed to get the current directory") - } else { - tmpdir.path().to_owned() - }; - - #[allow(deprecated)] - let path = TempPath::from_path("foo"); - assert_eq!(&*path, cwd.join("foo")); - - #[allow(deprecated)] - let path = TempPath::from_path(""); - assert_eq!(&*path, Path::new("")); TempPath::try_from_path("").expect_err("empty paths should fail"); } From ce86ac235d175963cc902713e15fe1323d246915 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 10:52:16 +0700 Subject: [PATCH 06/16] 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 +++--- src/lib.rs | 29 ----------------------------- tests/namedtempfile.rs | 8 +++++++- 4 files changed, 14 insertions(+), 34 deletions(-) diff --git a/src/dir/mod.rs b/src/dir/mod.rs index 84815e80a..7976db6c2 100644 --- a/src/dir/mod.rs +++ b/src/dir/mod.rs @@ -474,7 +474,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 01aabb4fe..c172d9a09 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -386,13 +386,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() } @@ -476,7 +476,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/src/lib.rs b/src/lib.rs index 3271ac622..1e1de6f99 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -81,35 +81,6 @@ //! create temporary a file (when the `getrandom` feature is enabled as it is by default on all //! major platforms). //! -//! ## Early drop pitfall -//! -//! Because `TempDir` and `NamedTempFile` rely on their destructors for cleanup, this can lead -//! to an unexpected early removal of the directory/file, usually when working with APIs which are -//! generic over `AsRef`. Consider the following example: -//! -//! ```no_run -//! use tempfile::tempdir; -//! use std::process::Command; -//! -//! // Create a directory inside of `env::temp_dir()`. -//! let temp_dir = tempdir()?; -//! -//! // Spawn the `touch` command inside the temporary directory and collect the exit status -//! // Note that `temp_dir` is **not** moved into `current_dir`, but passed as a reference -//! let exit_status = Command::new("touch").arg("tmp").current_dir(&temp_dir).status()?; -//! assert!(exit_status.success()); -//! -//! # Ok::<(), std::io::Error>(()) -//! ``` -//! -//! This works because a reference to `temp_dir` is passed to `current_dir`, resulting in the -//! destructor of `temp_dir` being run after the `Command` has finished execution. Moving the -//! `TempDir` into the `current_dir` call would result in the `TempDir` being converted into -//! an internal representation, with the original value being dropped and the directory thus -//! being deleted, before the command can be executed. -//! -//! The `touch` command would fail with an `No such file or directory` error. -//! //! ## Examples //! //! Create a temporary file and write some data into it: diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs index 8126ff627..0c8d7c34b 100644 --- a/tests/namedtempfile.rs +++ b/tests/namedtempfile.rs @@ -363,7 +363,13 @@ fn test_write_after_close() { configure_wasi_temp_dir(); 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 42561744d46e5aa9383263ba264211ec513c72be Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 10:54:36 +0700 Subject: [PATCH 07/16] 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 c172d9a09..9ad956cec 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -1039,12 +1039,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 b372b8a08055ac3d7ad74d281f9b81308a2045df Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 10:54:52 +0700 Subject: [PATCH 08/16] 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 1cacacebe..a2b663009 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 1e1de6f99..e6e490059 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -375,9 +375,9 @@ impl<'a, 'b> Builder<'a, 'b> { /// permissions of the created file may be more restrictive (but never more permissive) than the /// ones you specified. /// - /// 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. /// /// ## WASI /// From d1b8eeaffdf540a85309971f302593dc9fcc8b24 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 10:58:13 +0700 Subject: [PATCH 09/16] 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 | 60 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e6e490059..b7e8c0fd8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -164,6 +164,8 @@ 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, Permissions}; @@ -186,29 +188,22 @@ pub use crate::spooled::{SpooledData, SpooledTempFile, spooled_tempfile, spooled /// 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, disable_cleanup: 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, - disable_cleanup: false, - } + Builder::new() } } -impl<'a, 'b> Builder<'a, 'b> { +impl<'a> Builder<'a> { /// Create a new `Builder`. /// /// # Examples @@ -275,14 +270,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, + disable_cleanup: false, + } } /// Set a custom filename prefix. /// /// Path separators are legal but not advisable. - /// Default: `.tmp`. + /// Default: `tmp`. /// /// # Examples /// @@ -295,7 +297,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 } @@ -314,8 +316,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 } @@ -333,7 +335,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 } @@ -352,7 +354,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 } @@ -428,7 +430,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// # } /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn permissions(&mut self, permissions: Permissions) -> &mut Self { + pub const fn permissions(&mut self, permissions: Permissions) -> &mut Self { self.permissions = Some(permissions); self } @@ -458,7 +460,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// .tempfile()?; /// # Ok::<(), std::io::Error>(()) /// ``` - pub fn disable_cleanup(&mut self, disable_cleanup: bool) -> &mut Self { + pub const fn disable_cleanup(&mut self, disable_cleanup: bool) -> &mut Self { self.disable_cleanup = disable_cleanup; self } @@ -520,8 +522,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( @@ -586,8 +588,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.disable_cleanup), ) @@ -708,8 +710,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 5323e39450a4da6a8cfeba6e49b802d0a493186e Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 11:00:35 +0700 Subject: [PATCH 10/16] 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 b7e8c0fd8..5ced97d9a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -283,7 +283,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 @@ -303,7 +303,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 722b7391f..8dda0dc3a 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( @@ -64,7 +72,7 @@ pub fn create_helper( } let _ = i; // avoid unused variable warning for the above. - 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 @@ -80,3 +88,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 4d0b7d48aa545d5a9de5d2fdfc19de9c8f34e376 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 10 Nov 2024 12:24:33 +0700 Subject: [PATCH 11/16] 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 5ced97d9a..5e2c24f7f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -170,6 +170,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; @@ -596,11 +597,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`] /// @@ -609,8 +612,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 @@ -626,8 +627,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()); /// } /// @@ -635,7 +636,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>(()) /// ``` @@ -672,7 +673,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>(()) /// ``` @@ -682,7 +683,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) } @@ -699,13 +700,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( @@ -715,10 +716,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.disable_cleanup), )) }, ) } } + +/// 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 5672e2c24571733eb6541b389ded26a5b791d462 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 19 Nov 2024 11:57:30 -0800 Subject: [PATCH 12/16] feat: update env --- src/env.rs | 66 +++++++++++++++++++++++++++++++---------- src/file/imp/unix.rs | 2 +- src/file/imp/windows.rs | 2 +- src/lib.rs | 7 ++--- 4 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/env.rs b/src/env.rs index 5e70c4a01..10a858619 100644 --- a/src/env.rs +++ b/src/env.rs @@ -1,13 +1,20 @@ use std::env; +use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; #[cfg(doc)] use crate::{Builder, tempdir_in, tempfile_in}; // 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 @@ -18,17 +25,18 @@ static DEFAULT_TEMPDIR: OnceLock = OnceLock::new(); /// should instead use the `_in` variants of the various temporary file/directory constructors /// ([`tempdir_in`], [`tempfile_in`], the so-named functions on [`Builder`], etc.). /// -/// 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), + } } /// Returns the default temporary directory, used for both temporary directories and files if no @@ -37,11 +45,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 overridden by a call to [`override_temp_dir`]. /// -/// **NOTE:** This function does not 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 not 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 b7b63e127..4c68d16ce 100644 --- a/src/file/imp/unix.rs +++ b/src/file/imp/unix.rs @@ -71,7 +71,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 0c2bdf836..a8b0a253b 100644 --- a/src/file/imp/windows.rs +++ b/src/file/imp/windows.rs @@ -42,7 +42,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 5e2c24f7f..7bcc950ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -164,7 +164,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; @@ -523,7 +522,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| { @@ -589,7 +588,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.disable_cleanup), @@ -711,7 +710,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 7e10f3d2ad3308d262f7ce367c609269f5a4fcd2 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 30 Jan 2025 13:10:03 -0800 Subject: [PATCH 13/16] feat: drop once_cell dependency --- Cargo.toml | 2 -- src/env.rs | 10 ++-------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 40ab7b239..7482d752e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,8 +20,6 @@ include = ["CHANGELOG.md", "Cargo.toml", "LICENSE-*", "README.md", "src/**/*.rs" [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, <0.5", default-features = false, optional = true } diff --git a/src/env.rs b/src/env.rs index 10a858619..5e858b810 100644 --- a/src/env.rs +++ b/src/env.rs @@ -1,18 +1,12 @@ use std::env; use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; +use std::sync::{LazyLock, OnceLock}; #[cfg(doc)] use crate::{Builder, tempdir_in, tempfile_in}; -// 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 ENV_TEMPDIR: LazyLock = LazyLock::new(env::temp_dir); static DEFAULT_TEMPDIR: OnceLock = OnceLock::new(); static DEFAULT_PREFIX: OnceLock = OnceLock::new(); From 9b70f8df55b372ae5031d8c6ef044b61ba4fd606 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sat, 8 Feb 2025 15:20:02 -0800 Subject: [PATCH 14/16] 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 8dda0dc3a..4118b5c76 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 f510afd478a1c6e4ecbcedbc4d4ed7d769644b7b Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sat, 10 May 2025 15:28:35 -0400 Subject: [PATCH 15/16] feat: allow const spooled temporary files Cursors are stable in rust as of 1.79. This change lets us keep a global scratch file behind a Mutex. --- src/spooled.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/spooled.rs b/src/spooled.rs index 57ba9bead..a67e65122 100644 --- a/src/spooled.rs +++ b/src/spooled.rs @@ -64,7 +64,7 @@ pub struct SpooledTempFile { /// # Ok::<(), std::io::Error>(()) /// ``` #[inline] -pub fn spooled_tempfile(max_size: usize) -> SpooledTempFile { +pub const fn spooled_tempfile(max_size: usize) -> SpooledTempFile { SpooledTempFile::new(max_size) } @@ -94,7 +94,7 @@ fn cursor_to_tempfile(cursor: &Cursor>, p: &Option) -> io::Resu impl SpooledTempFile { /// Construct a new [`SpooledTempFile`]. #[must_use] - pub fn new(max_size: usize) -> SpooledTempFile { + pub const fn new(max_size: usize) -> SpooledTempFile { SpooledTempFile { max_size, dir: None, @@ -247,3 +247,18 @@ impl Seek for SpooledTempFile { } } } + +#[cfg(test)] +mod test { + use super::SpooledTempFile; + use std::io::Write; + use std::sync::Mutex; + + // This is fine because `tempfile` doesn't rely on drop. + static TEMPFILE: Mutex = Mutex::new(SpooledTempFile::new(1024)); + + #[test] + fn test_tempfile() { + TEMPFILE.lock().unwrap().write_all(b"foo").unwrap(); + } +} From 94c5614341338ae2fe25aa22c4ee5d6d0027a566 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 29 Aug 2025 10:47:01 -0700 Subject: [PATCH 16/16] fix: avoid panicing on platforms with no sane default tempdir fixes #365 --- src/env.rs | 24 ++++++++++++++++++------ src/file/mod.rs | 2 +- src/lib.rs | 6 +++--- tests/env.rs | 4 ++-- tests/namedtempfile.rs | 4 ++-- tests/spooled.rs | 2 +- 6 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/env.rs b/src/env.rs index 5e858b810..ba577e650 100644 --- a/src/env.rs +++ b/src/env.rs @@ -1,12 +1,18 @@ -use std::env; use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; use std::sync::{LazyLock, OnceLock}; +use std::{env, io}; #[cfg(doc)] use crate::{Builder, tempdir_in, tempfile_in}; -static ENV_TEMPDIR: LazyLock = LazyLock::new(env::temp_dir); +static ENV_TEMPDIR: LazyLock> = + // Only call env::temp_dir() on platforms known to not panic. + LazyLock::new(if cfg!(any(unix, windows, target_os = "hermit")) { + || Some(env::temp_dir()) + } else { + || None + }); static DEFAULT_TEMPDIR: OnceLock = OnceLock::new(); static DEFAULT_PREFIX: OnceLock = OnceLock::new(); @@ -36,16 +42,22 @@ pub fn override_temp_dir(path: impl Into) -> Result<&'static Path, &'st /// Returns the default temporary directory, used for both temporary directories and files if no /// directory is explicitly specified. /// -/// This function simply delegates to [`std::env::temp_dir`] unless the default temporary directory -/// has been overridden by a call to [`override_temp_dir`]. +/// Unless the default temporary directory has been overridden by a call to [`override_temp_dir`], +/// this function delegates to [`std::env::temp_dir`] (when supported by the platform). It returns +/// an error on platforms with no default temporary directory (e.g., WASI/WASM) unless +/// [`override_temp_dir`] has already been called to set the temporary directory. /// /// **NOTE:** /// /// 1. This function does not 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) +pub fn temp_dir() -> io::Result<&'static Path> { + DEFAULT_TEMPDIR + .get() + .or_else(|| ENV_TEMPDIR.as_ref()) + .map(|a| &**a) + .ok_or_else(|| io::Error::other("no temporary directory configured")) } /// Override the default prefix for new temporary files (defaults to "tmp"). This function changes diff --git a/src/file/mod.rs b/src/file/mod.rs index 9ad956cec..209459829 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -49,7 +49,7 @@ mod imp; /// # Ok::<(), std::io::Error>(()) /// ``` pub fn tempfile() -> io::Result { - tempfile_in(env::temp_dir()) + tempfile_in(env::temp_dir()?) } /// Create a new temporary file in the specified directory. Also see [`tempfile`]. diff --git a/src/lib.rs b/src/lib.rs index 7bcc950ce..0045a3241 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -491,7 +491,7 @@ impl<'a> Builder<'a> { /// [security]: struct.NamedTempFile.html#security /// [resource-leaking]: struct.NamedTempFile.html#resource-leaking pub fn tempfile(&self) -> io::Result { - self.tempfile_in(env::temp_dir()) + self.tempfile_in(env::temp_dir()?) } /// Create the named temporary file in the specified directory. @@ -560,7 +560,7 @@ impl<'a> Builder<'a> { /// /// [resource-leaking]: struct.TempDir.html#resource-leaking pub fn tempdir(&self) -> io::Result { - self.tempdir_in(env::temp_dir()) + self.tempdir_in(env::temp_dir()?) } /// Attempts to make a temporary directory inside of `dir`. @@ -684,7 +684,7 @@ impl<'a> Builder<'a> { where F: FnMut(&MakeParams<'_>) -> io::Result, { - self.make_in(env::temp_dir(), f) + self.make_in(env::temp_dir()?, f) } /// This is the same as [`Builder::make`], except `dir` is used as the base diff --git a/tests/env.rs b/tests/env.rs index 47317a8f2..3e6738af7 100644 --- a/tests/env.rs +++ b/tests/env.rs @@ -5,11 +5,11 @@ use std::path::Path; #[test] fn test_override_temp_dir() { #[cfg(not(target_os = "wasi"))] - assert_eq!(tempfile::env::temp_dir(), std::env::temp_dir()); + assert_eq!(tempfile::env::temp_dir().unwrap(), std::env::temp_dir()); let new_tmp = Path::new("/tmp/override"); tempfile::env::override_temp_dir(new_tmp).unwrap(); - assert_eq!(tempfile::env::temp_dir(), new_tmp); + assert_eq!(tempfile::env::temp_dir().unwrap(), new_tmp); let new_tmp2 = Path::new("/tmp/override2"); tempfile::env::override_temp_dir(new_tmp2).expect_err("override should only be possible once"); diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs index 0c8d7c34b..bd1887037 100644 --- a/tests/namedtempfile.rs +++ b/tests/namedtempfile.rs @@ -85,7 +85,7 @@ fn test_persist() { let mut tmpfile = NamedTempFile::new().unwrap(); let old_path = tmpfile.path().to_path_buf(); - let persist_path = env::temp_dir().join("persisted_temporary_file"); + let persist_path = env::temp_dir().unwrap().join("persisted_temporary_file"); write!(tmpfile, "abcde").unwrap(); { assert!(exists(&old_path)); @@ -233,7 +233,7 @@ fn test_temppath_persist() { let tmppath = tmpfile.into_temp_path(); let old_path = tmppath.to_path_buf(); - let persist_path = env::temp_dir().join("persisted_temppath_file"); + let persist_path = env::temp_dir().unwrap().join("persisted_temppath_file"); { assert!(exists(&old_path)); diff --git a/tests/spooled.rs b/tests/spooled.rs index 50529d963..d641927c4 100644 --- a/tests/spooled.rs +++ b/tests/spooled.rs @@ -43,7 +43,7 @@ fn test_custom_dir() { configure_wasi_temp_dir(); { - let mut t = spooled_tempfile_in(10, env::temp_dir()); + let mut t = spooled_tempfile_in(10, env::temp_dir().unwrap()); t.roll() .expect("failed to roll temp file in a specified directory"); }