diff --git a/Cargo.toml b/Cargo.toml index c5729c8e7..6124a97b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ authors = [ "Steven Allen ", "The Rust Project Developers", "Ashley Mannix ", - "Jason White ", + "Jason White ", ] documentation = "https://docs.rs/tempfile" edition = "2018" diff --git a/NEWS b/NEWS index c28442426..cd984c960 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,13 @@ +3.4.0 (WIP) +===== + +Features: + + * Add `Builder::make` and `Builder::make_in` for generalized temp file + creation. + * Add `NamedTempFile::from_parts` to complement `NamedTempFile::into_parts`. + * Add generic parameter to `NamedTempFile` to support wrapping non-File types. + 3.3.0 ===== diff --git a/src/file/mod.rs b/src/file/mod.rs index b859ced79..1f4eed956 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -478,18 +478,18 @@ impl AsRef for TempPath { /// [`NamedTempFile::new_in()`]: #method.new_in /// [`std::env::temp_dir()`]: https://doc.rust-lang.org/std/env/fn.temp_dir.html /// [`std::process::exit()`]: http://doc.rust-lang.org/std/process/fn.exit.html -pub struct NamedTempFile { +pub struct NamedTempFile { path: TempPath, - file: File, + file: F, } -impl fmt::Debug for NamedTempFile { +impl fmt::Debug for NamedTempFile { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "NamedTempFile({:?})", self.path) } } -impl AsRef for NamedTempFile { +impl AsRef for NamedTempFile { #[inline] fn as_ref(&self) -> &Path { self.path() @@ -497,41 +497,46 @@ impl AsRef for NamedTempFile { } /// Error returned when persisting a temporary file fails. -#[derive(Debug)] -pub struct PersistError { +pub struct PersistError { /// The underlying IO error. pub error: io::Error, /// The temporary file that couldn't be persisted. - pub file: NamedTempFile, + pub file: NamedTempFile, +} + +impl fmt::Debug for PersistError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "PersistError({:?})", self.error) + } } -impl From for io::Error { +impl From> for io::Error { #[inline] - fn from(error: PersistError) -> io::Error { + fn from(error: PersistError) -> io::Error { error.error } } -impl From for NamedTempFile { +impl From> for NamedTempFile { #[inline] - fn from(error: PersistError) -> NamedTempFile { + fn from(error: PersistError) -> NamedTempFile { error.file } } -impl fmt::Display for PersistError { +impl fmt::Display for PersistError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "failed to persist temporary file: {}", self.error) } } -impl error::Error for PersistError { +impl error::Error for PersistError { fn source(&self) -> Option<&(dyn error::Error + 'static)> { Some(&self.error) } } -impl NamedTempFile { +impl NamedTempFile { /// Create a new named temporary file. /// /// See [`Builder`] for more configuration. @@ -601,7 +606,9 @@ impl NamedTempFile { pub fn new_in>(dir: P) -> io::Result { Builder::new().tempfile_in(dir) } +} +impl NamedTempFile { /// Get the temporary file's path. /// /// # Security @@ -711,7 +718,7 @@ impl NamedTempFile { /// ``` /// /// [`PersistError`]: struct.PersistError.html - pub fn persist>(self, new_path: P) -> Result { + pub fn persist>(self, new_path: P) -> Result> { let NamedTempFile { path, file } = self; match path.persist(new_path) { Ok(_) => Ok(file), @@ -764,7 +771,7 @@ impl NamedTempFile { /// # Ok(()) /// # } /// ``` - pub fn persist_noclobber>(self, new_path: P) -> Result { + pub fn persist_noclobber>(self, new_path: P) -> Result> { let NamedTempFile { path, file } = self; match path.persist_noclobber(new_path) { Ok(_) => Ok(file), @@ -808,7 +815,7 @@ impl NamedTempFile { /// ``` /// /// [`PathPersistError`]: struct.PathPersistError.html - pub fn keep(self) -> Result<(File, PathBuf), PersistError> { + pub fn keep(self) -> Result<(F, PathBuf), PersistError> { let (file, path) = (self.file, self.path); match path.keep() { Ok(path) => Ok((file, path)), @@ -819,6 +826,49 @@ impl NamedTempFile { } } + /// Get a reference to the underlying file. + pub fn as_file(&self) -> &F { + &self.file + } + + /// Get a mutable reference to the underlying file. + pub fn as_file_mut(&mut self) -> &mut F { + &mut self.file + } + + /// Convert the temporary file into a `std::fs::File`. + /// + /// The inner file will be deleted. + pub fn into_file(self) -> F { + self.file + } + + /// Closes the file, leaving only the temporary file path. + /// + /// This is useful when another process must be able to open the temporary + /// file. + pub fn into_temp_path(self) -> TempPath { + self.path + } + + /// Converts the named temporary file into its constituent parts. + /// + /// Note: When the path is dropped, the file is deleted but the file handle + /// is still usable. + pub fn into_parts(self) -> (F, TempPath) { + (self.file, self.path) + } + + /// Creates a `NamedTempFile` from its constituent parts. + /// + /// This can be used with [`NamedTempFile::into_parts`] to reconstruct the + /// `NamedTempFile`. + pub fn from_parts(file: F, path: TempPath) -> Self { + Self { file, path } + } +} + +impl NamedTempFile { /// Securely reopen the temporary file. /// /// This function is useful when you need multiple independent handles to @@ -858,54 +908,24 @@ impl NamedTempFile { imp::reopen(self.as_file(), NamedTempFile::path(self)) .with_err_path(|| NamedTempFile::path(self)) } - - /// Get a reference to the underlying file. - pub fn as_file(&self) -> &File { - &self.file - } - - /// Get a mutable reference to the underlying file. - pub fn as_file_mut(&mut self) -> &mut File { - &mut self.file - } - - /// Convert the temporary file into a `std::fs::File`. - /// - /// The inner file will be deleted. - pub fn into_file(self) -> File { - self.file - } - - /// Closes the file, leaving only the temporary file path. - /// - /// This is useful when another process must be able to open the temporary - /// file. - pub fn into_temp_path(self) -> TempPath { - self.path - } - - /// Converts the named temporary file into its constituent parts. - /// - /// Note: When the path is dropped, the file is deleted but the file handle - /// is still usable. - pub fn into_parts(self) -> (File, TempPath) { - (self.file, self.path) - } } -impl Read for NamedTempFile { +impl Read for NamedTempFile { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.as_file_mut().read(buf).with_err_path(|| self.path()) } } -impl<'a> Read for &'a NamedTempFile { +impl<'a, F> Read for &'a NamedTempFile +where + &'a F: Read, +{ fn read(&mut self, buf: &mut [u8]) -> io::Result { self.as_file().read(buf).with_err_path(|| self.path()) } } -impl Write for NamedTempFile { +impl Write for NamedTempFile { fn write(&mut self, buf: &[u8]) -> io::Result { self.as_file_mut().write(buf).with_err_path(|| self.path()) } @@ -915,7 +935,10 @@ impl Write for NamedTempFile { } } -impl<'a> Write for &'a NamedTempFile { +impl<'a, F> Write for &'a NamedTempFile +where + &'a F: Write, +{ fn write(&mut self, buf: &[u8]) -> io::Result { self.as_file().write(buf).with_err_path(|| self.path()) } @@ -925,20 +948,26 @@ impl<'a> Write for &'a NamedTempFile { } } -impl Seek for NamedTempFile { +impl Seek for NamedTempFile { fn seek(&mut self, pos: SeekFrom) -> io::Result { self.as_file_mut().seek(pos).with_err_path(|| self.path()) } } -impl<'a> Seek for &'a NamedTempFile { +impl<'a, F> Seek for &'a NamedTempFile +where + &'a F: Seek, +{ fn seek(&mut self, pos: SeekFrom) -> io::Result { self.as_file().seek(pos).with_err_path(|| self.path()) } } #[cfg(unix)] -impl std::os::unix::io::AsRawFd for NamedTempFile { +impl std::os::unix::io::AsRawFd for NamedTempFile +where + F: std::os::unix::io::AsRawFd, +{ #[inline] fn as_raw_fd(&self) -> std::os::unix::io::RawFd { self.as_file().as_raw_fd() @@ -946,7 +975,10 @@ impl std::os::unix::io::AsRawFd for NamedTempFile { } #[cfg(windows)] -impl std::os::windows::io::AsRawHandle for NamedTempFile { +impl std::os::windows::io::AsRawHandle for NamedTempFile +where + F: std::os::windows::io::AsRawHandle, +{ #[inline] fn as_raw_handle(&self) -> std::os::windows::io::RawHandle { self.as_file().as_raw_handle() diff --git a/src/lib.rs b/src/lib.rs index c38ca7b87..7711e29f6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -534,4 +534,155 @@ impl<'a, 'b> Builder<'a, 'b> { util::create_helper(dir, self.prefix, self.suffix, self.random_len, dir::create) } + + /// 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 [`std::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`] + /// + /// This can be helpful for taking full control over the file creation, but + /// leaving the temporary file path construction up to the library. This + /// 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 + /// [`NamedTempFile`], but with additional caveats. Specifically, it is up + /// to the closure to ensure that the file does not exist and that such a + /// check is *atomic*. Otherwise, a [time-of-check to time-of-use + /// bug][TOCTOU] could be introduced. + /// + /// For example, the following is **not** secure: + /// + /// ``` + /// # use std::io; + /// # use std::fs::File; + /// # fn main() { + /// # if let Err(_) = run() { + /// # ::std::process::exit(1); + /// # } + /// # } + /// # fn run() -> Result<(), io::Error> { + /// # use tempfile::Builder; + /// // This is NOT secure! + /// let tempfile = Builder::new().make(|path| { + /// if path.is_file() { + /// return Err(io::ErrorKind::AlreadyExists.into()); + /// } + /// + /// // Between the check above and the usage below, an attacker could + /// // have replaced `path` with another file, which would get truncated + /// // by `File::create`. + /// + /// File::create(path) + /// })?; + /// # Ok(()) + /// # } + /// ``` + /// Note that simply using [`std::fs::File::create`] alone is not correct + /// because it does not fail if the file already exists: + /// ``` + /// # use std::io; + /// # use std::fs::File; + /// # fn main() { + /// # if let Err(_) = run() { + /// # ::std::process::exit(1); + /// # } + /// # } + /// # fn run() -> Result<(), io::Error> { + /// # use tempfile::Builder; + /// // This could overwrite an existing file! + /// let tempfile = Builder::new().make(|path| File::create(path))?; + /// # Ok(()) + /// # } + /// ``` + /// For creating regular temporary files, use [`Builder::tempfile`] instead + /// to avoid these problems. This function is meant to enable more exotic + /// use-cases. + /// + /// # Resource leaking + /// + /// See [the resource leaking][resource-leaking] docs on `NamedTempFile`. + /// + /// # Errors + /// + /// If the closure returns any error besides + /// [`std::io::ErrorKind::AlreadyExists`] or + /// [`std::io::ErrorKind::AddrInUse`], then `Err` is returned. + /// + /// # Examples + /// ``` + /// # use std::io; + /// # fn main() { + /// # if let Err(_) = run() { + /// # ::std::process::exit(1); + /// # } + /// # } + /// # fn run() -> Result<(), io::Error> { + /// # use tempfile::Builder; + /// # #[cfg(unix)] + /// use std::os::unix::net::UnixListener; + /// # #[cfg(unix)] + /// let tempsock = Builder::new().make(|path| UnixListener::bind(path))?; + /// # Ok(()) + /// # } + /// ``` + /// + /// [TOCTOU]: https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use + /// [security]: struct.NamedTempFile.html#security + /// [resource-leaking]: struct.NamedTempFile.html#resource-leaking + pub fn make(&self, f: F) -> io::Result> + where + F: FnMut(&Path) -> io::Result, + { + self.make_in(&env::temp_dir(), f) + } + + /// This is the same as [`Builder::make`], except `dir` is used as the base + /// directory for the temporary file path. + /// + /// See [`Builder::make`] for more details and security implications. + /// + /// # Examples + /// ``` + /// # use std::io; + /// # fn main() { + /// # if let Err(_) = run() { + /// # ::std::process::exit(1); + /// # } + /// # } + /// # fn run() -> Result<(), io::Error> { + /// # use tempfile::Builder; + /// # #[cfg(unix)] + /// use std::os::unix::net::UnixListener; + /// # #[cfg(unix)] + /// let tempsock = Builder::new().make_in("./", |path| UnixListener::bind(path))?; + /// # Ok(()) + /// # } + /// ``` + pub fn make_in(&self, dir: P, mut f: F) -> io::Result> + where + F: FnMut(&Path) -> io::Result, + P: AsRef, + { + util::create_helper( + dir.as_ref(), + self.prefix, + self.suffix, + self.random_len, + move |path| { + Ok(NamedTempFile::from_parts( + f(&path)?, + TempPath::from_path(path), + )) + }, + ) + } } diff --git a/src/util.rs b/src/util.rs index 8c91b9c69..5e95e5fcc 100644 --- a/src/util.rs +++ b/src/util.rs @@ -21,10 +21,10 @@ pub fn create_helper( prefix: &OsStr, suffix: &OsStr, random_len: usize, - f: F, + mut f: F, ) -> io::Result where - F: Fn(PathBuf) -> io::Result, + F: FnMut(PathBuf) -> io::Result, { let num_retries = if random_len != 0 { crate::NUM_RETRIES @@ -36,6 +36,9 @@ where let path = base.join(tmpname(prefix, suffix, random_len)); return match f(path) { Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists => continue, + // AddrInUse can happen if we're creating a UNIX domain socket and + // the path already exists. + Err(ref e) if e.kind() == io::ErrorKind::AddrInUse => continue, res => res, }; } diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs index d2c7da22b..cfdee337a 100644 --- a/tests/namedtempfile.rs +++ b/tests/namedtempfile.rs @@ -298,6 +298,18 @@ fn test_into_parts() { assert_eq!("abcdefgh", buf); } +#[test] +fn test_from_parts() { + let mut file = NamedTempFile::new().unwrap(); + write!(file, "abcd").expect("write failed"); + + let (file, temp_path) = file.into_parts(); + + let file = NamedTempFile::from_parts(file, temp_path); + + assert!(file.path().exists()); +} + #[test] fn test_keep() { let mut tmpfile = NamedTempFile::new().unwrap(); @@ -326,3 +338,104 @@ fn test_keep() { } std::fs::remove_file(&path).unwrap(); } + +#[test] +fn test_make() { + let tmpfile = Builder::new().make(|path| File::create(path)).unwrap(); + + assert!(tmpfile.path().is_file()); +} + +#[test] +fn test_make_in() { + let tmp_dir = tempdir().unwrap(); + + let tmpfile = Builder::new() + .make_in(tmp_dir.path(), |path| File::create(path)) + .unwrap(); + + assert!(tmpfile.path().is_file()); + assert_eq!(tmpfile.path().parent(), Some(tmp_dir.path())); +} + +#[test] +fn test_make_fnmut() { + let mut count = 0; + + // Show that an FnMut can be used. + let tmpfile = Builder::new() + .make(|path| { + count += 1; + File::create(path) + }) + .unwrap(); + + assert!(tmpfile.path().is_file()); +} + +#[cfg(unix)] +#[test] +fn test_make_uds() { + use std::os::unix::net::UnixListener; + + let temp_sock = Builder::new() + .prefix("tmp") + .suffix(".sock") + .rand_bytes(12) + .make(|path| UnixListener::bind(path)) + .unwrap(); + + assert!(temp_sock.path().exists()); +} + +#[cfg(unix)] +#[test] +fn test_make_uds_conflict() { + use std::os::unix::net::UnixListener; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::Arc; + + // Check that retries happen correctly by racing N different threads. + + const NTHREADS: usize = 20; + + // The number of times our callback was called. + let tries = Arc::new(AtomicUsize::new(0)); + + let mut threads = Vec::with_capacity(NTHREADS); + + for _ in 0..NTHREADS { + let tries = tries.clone(); + threads.push(std::thread::spawn(move || { + // Ensure that every thread uses the same seed so we are guaranteed + // to retry. Note that fastrand seeds are thread-local. + fastrand::seed(42); + + Builder::new() + .prefix("tmp") + .suffix(".sock") + .rand_bytes(12) + .make(|path| { + tries.fetch_add(1, Ordering::Relaxed); + UnixListener::bind(path) + }) + })); + } + + // Join all threads, but don't drop the temp file yet. Otherwise, we won't + // get a deterministic number of `tries`. + let sockets: Vec<_> = threads + .into_iter() + .map(|thread| thread.join().unwrap().unwrap()) + .collect(); + + // Number of tries is exactly equal to (n*(n+1))/2. + assert_eq!( + tries.load(Ordering::Relaxed), + (NTHREADS * (NTHREADS + 1)) / 2 + ); + + for socket in sockets { + assert!(socket.path().exists()); + } +}