Repository: Stebalien/tempfile
Pull request #80 (2019-02-10 23:14:50)

Use absolute paths
When looking into #40, I realized we weren't actually _doing_ what I claimed. Except for never-named tempfiles on Linux and auto-removed temporary files on Windows, we need to use absolute path names to guard against changes in the current working directory.

Unfortunately, this is a **[breaking change]** on some unix-like OSs. It's possible to:

1. Chdir to a directory.
2. Chmod the parent directory to remove the search (`x`) permission.

This will break any uses of absolute path names. The solution is the one described in #40: open the current directory, use `openat` on the relative path, and then close the current directory. However:

1. This will only help with unnamed temporary files unless we're willing to _hold_ the file directory open (probably not the best idea).
2. This will require two filesystem extra syscalls (instead of one as with getcwd).


Patch:
diff --git a/src/file/imp/unix.rs b/src/file/imp/unix.rs
index 03f3714..b66c914 100644
--- a/src/file/imp/unix.rs
+++ b/src/file/imp/unix.rs
@@ -1,5 +1,6 @@
 #[cfg(not(target_os = "redox"))]
 use libc::{c_char, c_int, link, rename, unlink, O_CLOEXEC, O_CREAT, O_EXCL, O_RDWR};
+use std::env;
 use std::ffi::{CString, OsStr};
 use std::fs::{self, File, OpenOptions};
 use std::io;
@@ -64,6 +65,15 @@ pub fn create_named(path: &Path) -> io::Result<File> {
 }
 
 fn create_unlinked(path: &Path) -> io::Result<File> {
+    let tmp;
+    // shadow this to decrease the lifetime. It can't live longer than `tmp`.
+    let mut path = path;
+    if !path.is_absolute() {
+        let cur_dir = env::current_dir()?;
+        tmp = cur_dir.join(path);
+        path = &tmp;
+    }
+
     let f = create_named(path)?;
     // don't care whether the path has already been unlinked,
     // but perhaps there are some IO error conditions we should send up?
diff --git a/src/file/mod.rs b/src/file/mod.rs
index c16d8ec..d0307e5 100644
--- a/src/file/mod.rs
+++ b/src/file/mod.rs
@@ -795,7 +795,12 @@ impl std::os::windows::io::AsRawHandle for NamedTempFile {
     }
 }
 
-pub(crate) fn create_named(path: PathBuf) -> io::Result<NamedTempFile> {
+pub(crate) fn create_named(mut path: PathBuf) -> io::Result<NamedTempFile> {
+    // Make the path absolute. Otherwise, changing directories could cause us to
+    // delete the wrong file.
+    if !path.is_absolute() {
+        path = env::current_dir()?.join(path)
+    }
     imp::create_named(&path)
         .with_err_path(|| path.clone())
         .map(|file| NamedTempFile {


Test patch:
diff --git a/tests/namedtempfile.rs b/tests/namedtempfile.rs
index 4ded326..62cc253 100644
--- a/tests/namedtempfile.rs
+++ b/tests/namedtempfile.rs
@@ -207,3 +207,13 @@ fn test_write_after_close() {
     let path = NamedTempFile::new().unwrap().into_temp_path();
     File::create(path).unwrap().write_all(b"test").unwrap();
 }
+
+#[test]
+fn test_change_dir() {
+    env::set_current_dir(env::temp_dir()).unwrap();
+    let tmpfile = NamedTempFile::new_in(".").unwrap();
+    let path = env::current_dir().unwrap().join(tmpfile.path());
+    env::set_current_dir("/").unwrap();
+    drop(tmpfile);
+    assert!(!exists(path))
+}