Skip to content

Commit 8afa07a

Browse files
authored
fix: always use absolute paths (#323)
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.
1 parent 6ecd9f1 commit 8afa07a

File tree

4 files changed

+28
-17
lines changed

4 files changed

+28
-17
lines changed

src/file/mod.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,16 +1043,11 @@ impl<F: AsRawHandle> AsRawHandle for NamedTempFile<F> {
10431043
}
10441044

10451045
pub(crate) fn create_named(
1046-
mut path: PathBuf,
1046+
path: PathBuf,
10471047
open_options: &mut OpenOptions,
10481048
permissions: Option<&std::fs::Permissions>,
10491049
keep: bool,
10501050
) -> io::Result<NamedTempFile> {
1051-
// Make the path absolute. Otherwise, changing directories could cause us to
1052-
// delete the wrong file.
1053-
if !path.is_absolute() {
1054-
path = std::env::current_dir()?.join(path)
1055-
}
10561051
imp::create_named(&path, open_options, permissions)
10571052
.with_err_path(|| path.clone())
10581053
.map(|file| NamedTempFile {

src/lib.rs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -604,17 +604,13 @@ impl<'a, 'b> Builder<'a, 'b> {
604604
///
605605
/// [resource-leaking]: struct.TempDir.html#resource-leaking
606606
pub fn tempdir_in<P: AsRef<Path>>(&self, dir: P) -> io::Result<TempDir> {
607-
let storage;
608-
let mut dir = dir.as_ref();
609-
if !dir.is_absolute() {
610-
let cur_dir = std::env::current_dir()?;
611-
storage = cur_dir.join(dir);
612-
dir = &storage;
613-
}
614-
615-
util::create_helper(dir, self.prefix, self.suffix, self.random_len, |path| {
616-
dir::create(path, self.permissions.as_ref(), self.keep)
617-
})
607+
util::create_helper(
608+
dir.as_ref(),
609+
self.prefix,
610+
self.suffix,
611+
self.random_len,
612+
|path| dir::create(path, self.permissions.as_ref(), self.keep),
613+
)
618614
}
619615

620616
/// Attempts to create a temporary file (or file-like object) using the

src/util.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,16 @@ pub fn create_helper<R>(
2626
random_len: usize,
2727
mut f: impl FnMut(PathBuf) -> io::Result<R>,
2828
) -> io::Result<R> {
29+
// Make the path absolute. Otherwise, changing the current directory can invalidate a stored
30+
// path (causing issues when cleaning up temporary files.
31+
let mut base = &*base; // re-borrow to shrink lifetime
32+
let base_path_storage; // slot to store the absolute path, if necessary.
33+
if !base.is_absolute() {
34+
let cur_dir = std::env::current_dir()?;
35+
base_path_storage = cur_dir.join(base);
36+
base = &base_path_storage;
37+
}
38+
2939
let num_retries = if random_len != 0 {
3040
crate::NUM_RETRIES
3141
} else {

tests/namedtempfile.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,16 @@ fn test_change_dir() {
290290
assert!(!exists(path))
291291
}
292292

293+
#[test]
294+
fn test_change_dir_make() {
295+
std::env::set_current_dir(env::temp_dir()).unwrap();
296+
let tmpfile = Builder::new().make_in(".", |p| File::create(p)).unwrap();
297+
let path = std::env::current_dir().unwrap().join(tmpfile.path());
298+
std::env::set_current_dir("/").unwrap();
299+
drop(tmpfile);
300+
assert!(!exists(path))
301+
}
302+
293303
#[test]
294304
fn test_into_parts() {
295305
let mut file = NamedTempFile::new().unwrap();

0 commit comments

Comments
 (0)