Skip to content

Commit beb98fc

Browse files
committed
fix: resolve relative paths when constructing TempPath
1. Try to transform relative paths into absolute paths in `TempPath::from_path` and silently proceed when we can't. 2. Deprecate `TempPath::from_path` and add `TempPath::try_from_path` as a replacement that propegates errors. reported by @meng-xu-cs fixes #395
1 parent 929a112 commit beb98fc

File tree

2 files changed

+102
-16
lines changed

2 files changed

+102
-16
lines changed

src/file/mod.rs

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,19 +325,65 @@ impl TempPath {
325325
self.disable_cleanup = disable_cleanup
326326
}
327327

328-
/// Create a new TempPath from an existing path. This can be done even if no
329-
/// file exists at the given path.
328+
/// Create a new TempPath from an existing path. This can be done even if no file exists at the
329+
/// given path.
330330
///
331-
/// This is mostly useful for interacting with libraries and external
332-
/// components that provide files to be consumed or expect a path with no
333-
/// existing file to be given.
331+
/// This is mostly useful for interacting with libraries and external components that provide
332+
/// files to be consumed or expect a path with no existing file to be given.
333+
///
334+
/// When passed a relative path, this function first tries to make it absolute (relative to the
335+
/// current directory). If this fails, this function uses the relative path as-is.
336+
///
337+
/// **DEPRECATED:** Use [`TempPath::try_from_path`] instead to handle the case where looking up
338+
/// the current directory fails.
339+
#[deprecated = "use TempPath::try_from_path"]
334340
pub fn from_path(path: impl Into<PathBuf>) -> Self {
341+
let mut path = path.into();
342+
// Best effort to resolve a relative path. If we fail, we keep the path as-is to
343+
// preserve backwards compatibility.
344+
//
345+
// Ignore empty paths entirely. There's nothing we can do about them.
346+
if &path != "" && !path.is_absolute() {
347+
if let Ok(cur_dir) = std::env::current_dir() {
348+
path = cur_dir.join(path);
349+
}
350+
}
335351
Self {
336-
path: path.into().into_boxed_path(),
352+
path: path.into_boxed_path(),
337353
disable_cleanup: false,
338354
}
339355
}
340356

357+
/// Create a new TempPath from an existing path. This can be done even if no file exists at the
358+
/// given path.
359+
///
360+
/// This is mostly useful for interacting with libraries and external components that provide
361+
/// files to be consumed or expect a path with no existing file to be given.
362+
///
363+
/// Relative paths are resolved relative to the current working directory. If the current
364+
/// working directory cannot be determined, this function returns an error.
365+
///
366+
/// **NOTE:** this function does not check if the target path exists.
367+
pub fn try_from_path(path: impl Into<PathBuf>) -> io::Result<Self> {
368+
let mut path = path.into();
369+
if !path.is_absolute() {
370+
if &path == "" {
371+
return Err(io::Error::new(
372+
io::ErrorKind::InvalidInput,
373+
"cannot construct a TempPath from an empty path",
374+
));
375+
}
376+
let mut cwd = std::env::current_dir()?;
377+
cwd.push(path);
378+
path = cwd;
379+
};
380+
381+
Ok(Self {
382+
path: path.into_boxed_path(),
383+
disable_cleanup: false,
384+
})
385+
}
386+
341387
pub(crate) fn new(path: PathBuf, disable_cleanup: bool) -> Self {
342388
Self {
343389
path: path.into_boxed_path(),

tests/namedtempfile.rs

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::ffi::{OsStr, OsString};
44
use std::fs::File;
55
use std::io::{self, Read, Seek, SeekFrom, Write};
66
use std::path::{Path, PathBuf};
7+
use std::time::Duration;
78
use tempfile::{env, tempdir, Builder, NamedTempFile, TempPath};
89

910
fn exists<P: AsRef<Path>>(path: P) -> bool {
@@ -280,7 +281,7 @@ fn temp_path_from_existing() {
280281
File::create(&tmp_file_path_2).unwrap();
281282
assert!(tmp_file_path_2.exists(), "Test file 2 hasn't been created");
282283

283-
let tmp_path = TempPath::from_path(&tmp_file_path_1);
284+
let tmp_path = TempPath::try_from_path(&tmp_file_path_1).unwrap();
284285
assert!(
285286
tmp_file_path_1.exists(),
286287
"Test file has been deleted before dropping TempPath"
@@ -303,13 +304,49 @@ fn temp_path_from_argument_types() {
303304
// This just has to compile
304305
return;
305306

306-
TempPath::from_path("");
307-
TempPath::from_path(String::new());
308-
TempPath::from_path(OsStr::new(""));
309-
TempPath::from_path(OsString::new());
310-
TempPath::from_path(Path::new(""));
311-
TempPath::from_path(PathBuf::new());
312-
TempPath::from_path(PathBuf::new().into_boxed_path());
307+
TempPath::try_from_path("").unwrap();
308+
TempPath::try_from_path(String::new()).unwrap();
309+
TempPath::try_from_path(OsStr::new("")).unwrap();
310+
TempPath::try_from_path(OsString::new()).unwrap();
311+
TempPath::try_from_path(Path::new("")).unwrap();
312+
TempPath::try_from_path(PathBuf::new()).unwrap();
313+
TempPath::try_from_path(PathBuf::new().into_boxed_path()).unwrap();
314+
}
315+
316+
#[test]
317+
fn test_temp_path_resolve_missing_cwd() {
318+
let tmpdir = tempdir().unwrap();
319+
std::env::set_current_dir(&tmpdir).expect("failed to change to the temporary directory");
320+
tmpdir.close().unwrap();
321+
// It can sometimes take a bit for the OS to realize the CWD is removed. I'm not sure why, but
322+
// this test fails occasionally without this.
323+
while std::env::current_dir().is_ok() {
324+
std::thread::sleep(Duration::from_millis(1));
325+
}
326+
327+
#[allow(deprecated)]
328+
let path = TempPath::from_path("foo");
329+
assert_eq!(&*path, "foo");
330+
331+
TempPath::try_from_path("foo").expect_err("should have failed to make path absolute file");
332+
}
333+
334+
#[test]
335+
fn test_temp_path_resolve_existing_cwd() {
336+
let tmpdir = tempdir().unwrap();
337+
std::env::set_current_dir(&tmpdir).expect("failed to change to directory");
338+
339+
#[allow(deprecated)]
340+
let path = TempPath::from_path("foo");
341+
assert_eq!(&*path, tmpdir.path().join("foo"));
342+
343+
#[allow(deprecated)]
344+
let path = TempPath::from_path("");
345+
assert_eq!(&*path, "");
346+
347+
TempPath::try_from_path("").expect_err("empty paths should fail");
348+
349+
tmpdir.close().unwrap();
313350
}
314351

315352
#[test]
@@ -327,7 +364,7 @@ fn test_change_dir() {
327364
let dir_a = tempdir().unwrap();
328365
let dir_b = tempdir().unwrap();
329366

330-
std::env::set_current_dir(&dir_a).expect("failed to change to directory ~");
367+
std::env::set_current_dir(&dir_a).expect("failed to change to directory A");
331368
let tmpfile = NamedTempFile::new_in(".").unwrap();
332369
let path = std::env::current_dir().unwrap().join(tmpfile.path());
333370
std::env::set_current_dir(&dir_b).expect("failed to change to directory B");
@@ -574,7 +611,10 @@ fn test_reseed() {
574611
.open(path)
575612
.unwrap();
576613

577-
files.push(NamedTempFile::from_parts(f, TempPath::from_path(path)));
614+
files.push(NamedTempFile::from_parts(
615+
f,
616+
TempPath::try_from_path(path).unwrap(),
617+
));
578618
Err(io::Error::new(io::ErrorKind::AlreadyExists, "fake!"))
579619
});
580620

0 commit comments

Comments
 (0)