Repository: alexcrichton/tar-rs (language: rust)

Issue:
Permission error unpacking a directory hierarchy with write-protected folders
When unpacking an archive looking like this:

```
$ tar tvf test.tar.gz
dr-xr-xr-x abbe/staff        0 2021-01-30 17:48 read/
dr-xr-xr-x abbe/staff        0 2021-01-30 17:48 read/only/
-r--r--r-- abbe/staff        0 2021-01-30 17:48 read/only/dirs-above-me
```

using this code 

```rust
use std::fs::File;
use flate2::read::GzDecoder;
use tar::Archive;

fn main() -> Result<(), std::io::Error> {
    let path = "test.tar.gz";

    let tar_gz = File::open(path)?;
    let tar = GzDecoder::new(tar_gz);
    let mut archive = Archive::new(tar);
    archive.unpack(".")?;

    Ok(())
}
```

the result is:

```
Error: Custom { kind: PermissionDenied, error: TarError { desc: "failed to unpack `/Users/abbe/code/tar-permissions/read/only`", io: Custom { kind: PermissionDenied, error: "Permission denied (os error 13) when creating dir /Users/abbe/code/tar-permissions/read/only" } } }
```

This archive works fine to unpack with the `tar` command. This seems to come from the fact that permissions from the tar archive is applied to each folder as it is created and not as an "atomic" operation after unpack [here](https://github.com/alexcrichton/tar-rs/blob/6fa079849506b5044206ab65640fb0f940b69884/src/entry.rs#L443).


Discussion / hints:
Is there a workaround for this?
Not that I know of except packing it with different permissions.
Just found an additional `PermissionDenied` case. If your archive has a folder that has `drw-r--r--` permissions (notice how it is not +x, so you cannot `cd` into it), `archive.unpack` will fail. 

Fix patch:
diff --git a/src/archive.rs b/src/archive.rs
index 8c0b0894..8c333322 100644
--- a/src/archive.rs
+++ b/src/archive.rs
@@ -172,10 +172,22 @@ impl<'a> Archive<dyn Read + 'a> {
         // NotFound exception.
         let dst = &dst.canonicalize().unwrap_or(dst.to_path_buf());
 
+        // Delay any directory entries until the end (they will be created if needed by
+        // descendants), to ensure that directory permissions do not interfer with descendant
+        // extraction.
+        let mut directories = Vec::new();
         for entry in self._entries()? {
             let mut file = entry.map_err(|e| TarError::new("failed to iterate over archive", e))?;
-            file.unpack_in(dst)?;
+            if file.header().entry_type() == crate::EntryType::Directory {
+                directories.push(file);
+            } else {
+                file.unpack_in(dst)?;
+            }
+        }
+        for mut dir in directories {
+            dir.unpack_in(dst)?;
         }
+
         Ok(())
     }
 


Test patch:
diff --git a/tests/all.rs b/tests/all.rs
index 4a31cd35..93ec3e23 100644
--- a/tests/all.rs
+++ b/tests/all.rs
@@ -1149,3 +1149,29 @@ fn append_long_multibyte() {
         name.pop();
     }
 }
+
+#[test]
+fn read_only_directory_containing_files() {
+    let td = t!(TempBuilder::new().prefix("tar-rs").tempdir());
+
+    let mut b = Builder::new(Vec::<u8>::new());
+
+    let mut h = Header::new_gnu();
+    t!(h.set_path("dir/"));
+    h.set_size(0);
+    h.set_entry_type(EntryType::dir());
+    h.set_mode(0o444);
+    h.set_cksum();
+    t!(b.append(&h, "".as_bytes()));
+
+    let mut h = Header::new_gnu();
+    t!(h.set_path("dir/file"));
+    h.set_size(2);
+    h.set_entry_type(EntryType::file());
+    h.set_cksum();
+    t!(b.append(&h, "hi".as_bytes()));
+
+    let contents = t!(b.into_inner());
+    let mut ar = Archive::new(&contents[..]);
+    assert!(ar.unpack(td.path()).is_ok());
+}