From 4517271323ffaa4eba9fbbe572c6ebab839c16d8 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Sat, 8 Feb 2025 14:58:06 -0800
Subject: [PATCH] fix: always use absolute paths

We used absolute paths in the builder when making directories and in
`create_named` but this didn't cover `Builder::make_in` and technically
introduced a small race in some unnamed tempfile cases (i.e., when we
can't atomically create unnamed temporary files and need to create then
immediately delete them).

Instead, we now resolve the filename into an absolute path inside the
main create helper.
---
 src/file/mod.rs        |  7 +------
 src/lib.rs             | 18 +++++++-----------
 src/util.rs            | 10 ++++++++++
 tests/namedtempfile.rs | 10 ++++++++++
 4 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/file/mod.rs b/src/file/mod.rs
index 1cb238221..5907a79aa 100644
--- a/src/file/mod.rs
+++ b/src/file/mod.rs
@@ -1043,16 +1043,11 @@ impl<F: AsRawHandle> AsRawHandle for NamedTempFile<F> {
 }
 
 pub(crate) fn create_named(
-    mut path: PathBuf,
+    path: PathBuf,
     open_options: &mut OpenOptions,
     permissions: Option<&std::fs::Permissions>,
     keep: bool,
 ) -> io::Result<NamedTempFile> {
-    // Make the path absolute. Otherwise, changing directories could cause us to
-    // delete the wrong file.
-    if !path.is_absolute() {
-        path = std::env::current_dir()?.join(path)
-    }
     imp::create_named(&path, open_options, permissions)
         .with_err_path(|| path.clone())
         .map(|file| NamedTempFile {
diff --git a/src/lib.rs b/src/lib.rs
index 590bd8418..e0e050005 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -604,17 +604,13 @@ impl<'a, 'b> Builder<'a, 'b> {
     ///
     /// [resource-leaking]: struct.TempDir.html#resource-leaking
     pub fn tempdir_in<P: AsRef<Path>>(&self, dir: P) -> io::Result<TempDir> {
-        let storage;
-        let mut dir = dir.as_ref();
-        if !dir.is_absolute() {
-            let cur_dir = std::env::current_dir()?;
-            storage = cur_dir.join(dir);
-            dir = &storage;
-        }
-
-        util::create_helper(dir, self.prefix, self.suffix, self.random_len, |path| {
-            dir::create(path, self.permissions.as_ref(), self.keep)
-        })
+        util::create_helper(
+            dir.as_ref(),
+            self.prefix,
+            self.suffix,
+            self.random_len,
+            |path| dir::create(path, self.permissions.as_ref(), self.keep),
+        )
     }
 
     /// Attempts to create a temporary file (or file-like object) using the
diff --git a/src/util.rs b/src/util.rs
index 63518b583..98aa36212 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -26,6 +26,16 @@ pub fn create_helper<R>(
     random_len: usize,
     mut f: impl FnMut(PathBuf) -> io::Result<R>,
 ) -> io::Result<R> {
+    // Make the path absolute. Otherwise, changing the current directory can invalidate a stored
+    // path (causing issues when cleaning up temporary files.
+    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 = &base_path_storage;
+    }
+
     let num_retries = if random_len != 0 {
         crate::NUM_RETRIES
     } else {
diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs
index 8ef302697..783ff22c2 100644
--- a/tests/namedtempfile.rs
+++ b/tests/namedtempfile.rs
@@ -290,6 +290,16 @@ fn test_change_dir() {
     assert!(!exists(path))
 }
 
+#[test]
+fn test_change_dir_make() {
+    std::env::set_current_dir(env::temp_dir()).unwrap();
+    let tmpfile = Builder::new().make_in(".", |p| File::create(p)).unwrap();
+    let path = std::env::current_dir().unwrap().join(tmpfile.path());
+    std::env::set_current_dir("/").unwrap();
+    drop(tmpfile);
+    assert!(!exists(path))
+}
+
 #[test]
 fn test_into_parts() {
     let mut file = NamedTempFile::new().unwrap();