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 d8f8cb521..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.63" + - "${{ 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 diff --git a/Cargo.toml b/Cargo.toml index 5ab5c0a16..7482d752e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,8 +8,8 @@ authors = [ "Jason White ", ] documentation = "https://docs.rs/tempfile" -edition = "2021" -rust-version = "1.63" +edition = "2024" +rust-version = "1.85" homepage = "https://stebalien.com/projects/tempfile-rs/" keywords = ["tempfile", "tmpfile", "filesystem"] license = "MIT OR Apache-2.0" @@ -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 } @@ -41,7 +39,6 @@ doc-comment = "0.3" [features] default = ["getrandom"] -nightly = [] # DEPRECATED [package.metadata.docs.rs] rustdoc-args = ["--generate-link-to-definition"] 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`: 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..a2b663009 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; @@ -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/dir/mod.rs b/src/dir/mod.rs index 93a8d7ccb..7976db6c2 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; @@ -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 @@ -481,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/env.rs b/src/env.rs index 4276b9ad4..ba577e650 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}; +use std::sync::{LazyLock, OnceLock}; +use std::{env, io}; #[cfg(doc)] -use crate::{tempdir_in, tempfile_in, Builder}; - -// Once rust 1.70 is wide-spread (Debian stable), we can use OnceLock from stdlib. -use once_cell::sync::OnceCell as OnceLock; +use crate::{Builder, tempdir_in, tempfile_in}; +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(); /// 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,34 +25,63 @@ 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 /// 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:** /// -/// **NOTE:** This function does not check if the returned directory exists and/or is writable. -pub fn temp_dir() -> PathBuf { +/// 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() -> io::Result<&'static Path> { 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) + .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 +/// 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 987791e14..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), @@ -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..a8b0a253b 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; @@ -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/file/mod.rs b/src/file/mod.rs index d2b5ec208..209459829 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; @@ -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`]. @@ -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. /// @@ -415,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() } @@ -505,7 +476,7 @@ impl fmt::Debug for NamedTempFile { } } -impl AsRef for NamedTempFile { +impl AsRef for &NamedTempFile { #[inline] fn as_ref(&self) -> &Path { self.path() @@ -1068,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"))] diff --git a/src/lib.rs b/src/lib.rs index 5dff328a5..0045a3241 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: @@ -193,10 +164,12 @@ doc_comment::doctest!("../README.md"); 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; use std::path::Path; mod dir; @@ -207,37 +180,30 @@ 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)] -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 @@ -304,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`. + /// Path separators are not allowed. + /// Default: `tmp`. /// /// # Examples /// @@ -324,13 +297,13 @@ 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 } /// Set a custom filename suffix. /// - /// Path separators are legal but not advisable. + /// Path separators are not allowed. /// Default: empty. /// /// # Examples @@ -343,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 } @@ -362,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 } @@ -381,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 } @@ -404,9 +377,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 /// @@ -457,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 } @@ -487,17 +460,11 @@ 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 } - /// 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 @@ -524,7 +491,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// [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. @@ -555,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(env::default_prefix()), + self.suffix.unwrap_or(DEFAULT_SUFFIX.as_ref()), self.random_len, |path| { file::create_named( @@ -593,7 +560,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// /// [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`. @@ -621,19 +588,21 @@ 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(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), ) } /// 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`] /// @@ -642,8 +611,6 @@ impl<'a, 'b> Builder<'a, 'b> { /// 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 @@ -659,8 +626,8 @@ impl<'a, 'b> Builder<'a, 'b> { /// 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()); /// } /// @@ -668,7 +635,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// // have replaced `path` with another file, which would get truncated /// // by `File::create`. /// - /// File::create(path) + /// File::create(params) /// })?; /// # Ok::<(), std::io::Error>(()) /// ``` @@ -705,7 +672,7 @@ impl<'a, 'b> Builder<'a, 'b> { /// 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>(()) /// ``` @@ -715,9 +682,9 @@ impl<'a, 'b> Builder<'a, 'b> { /// [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) + self.make_in(env::temp_dir()?, f) } /// This is the same as [`Builder::make`], except `dir` is used as the base @@ -732,26 +699,60 @@ impl<'a, 'b> Builder<'a, 'b> { /// 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( dir.as_ref(), - self.prefix, - self.suffix, + self.prefix.unwrap_or(env::default_prefix()), + self.suffix.unwrap_or(DEFAULT_SUFFIX.as_ref()), 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() + } +} 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(); + } +} diff --git a/src/util.rs b/src/util.rs index 722b7391f..4118b5c76 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( @@ -31,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; } @@ -64,7 +71,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 +87,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"); +} 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 88e31bccd..bd1887037 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)] @@ -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)); @@ -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"); } @@ -387,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] @@ -637,7 +619,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() diff --git a/tests/spooled.rs b/tests/spooled.rs index 21cfeb7eb..d641927c4 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. @@ -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"); } 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, };