Skip to content

Commit

Permalink
fix: always use absolute paths
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Stebalien committed Feb 8, 2025
1 parent 6ecd9f1 commit 4517271
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 17 deletions.
7 changes: 1 addition & 6 deletions src/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 7 additions & 11 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions tests/namedtempfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 4517271

Please sign in to comment.