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

Issue:
Directory permissions
Seems like, that upon untar() the library is creating directories with the default rwxr-xr-x mode even if the tar itself contains different permissions.

Starting from here:
https://github.com/alexcrichton/tar-rs/blob/d7977e748aacadb8431293087eee4bdc99e225ed/src/entry.rs#L384

1. The code finds a directory entry
2. If the dir exists does nothing
3. Otherwise it creates it with the default permissions

A 4. step could be to set_mode the directory with the permissions of the tar entry. Or do I miss something?


Discussion / hints:
Makes sense to me! Seems like a reasonable change to have!
Thinking about it a bit:
1. All directories that need permission change have to be remembered
2. Permissions should be changed as last step of extraction

Otherwise there will be problems with the non writable (r-x) directories...
Hi, has any progress been made on this? Merging a fix would be awesome, just inquiring before trying to look into it, if not I'll try to put up a decent PR.

Thanks!
AFAIK no progress, but a PR would be most welcome!

Fix patch:
diff --git a/src/entry.rs b/src/entry.rs
index fdddf6fd..d07c987c 100644
--- a/src/entry.rs
+++ b/src/entry.rs
@@ -401,12 +401,12 @@ impl<'a> EntryFields<'a> {
         if prev.map(|m| m.is_dir()).unwrap_or(false) {
             return Ok(());
         }
-        return fs::create_dir(dst).map_err(|err| {
+        fs::create_dir(dst).map_err(|err| {
             Error::new(
                 err.kind(),
                 format!("{} when creating dir {}", err, dst.display()),
             )
-        });
+        })
     }
 
     /// Returns access to the header of this entry in the archive.
@@ -414,7 +414,9 @@ impl<'a> EntryFields<'a> {
         let kind = self.header.entry_type();
 
         if kind.is_dir() {
-            return self.unpack_dir(dst);
+            return self.unpack_dir(dst)
+                .and_then(|_| self.header.mode())
+                .and_then(|mode| set_perms(dst, mode, self.preserve_permissions));
         } else if kind.is_hard_link() || kind.is_symlink() {
             let src = match self.link_name()? {
                 Some(name) => name,
@@ -422,7 +424,7 @@ impl<'a> EntryFields<'a> {
                     return Err(other(&format!(
                         "hard link listed for {} but no link name found",
                         String::from_utf8_lossy(self.header.as_bytes())
-                    )))
+                    )));
                 }
             };
 
@@ -498,9 +500,7 @@ impl<'a> EntryFields<'a> {
         // Old BSD-tar compatibility.
         // Names that have a trailing slash should be treated as a directory.
         // Only applies to old headers.
-        if self.header.as_ustar().is_none()
-            && self.path_bytes().ends_with(b"/")
-        {
+        if self.header.as_ustar().is_none() && self.path_bytes().ends_with(b"/") {
             return self.unpack_dir(dst);
         }
 
@@ -540,7 +540,8 @@ impl<'a> EntryFields<'a> {
                 }
             }
             Ok(())
-        })().map_err(|e| {
+        })()
+        .map_err(|e| {
             let header = self.header.path_bytes();
             TarError::new(
                 &format!(


Test patch:
diff --git a/tests/entry.rs b/tests/entry.rs
index 2cba1bf6..5d3c8ca4 100644
--- a/tests/entry.rs
+++ b/tests/entry.rs
@@ -154,6 +154,32 @@ fn relative_link_deref_error() {
     assert!(File::open(td.path().join("foo").join("bar")).is_err());
 }
 
+#[test]
+#[cfg(unix)]
+fn directory_maintains_permissions() {
+    use ::std::os::unix::fs::PermissionsExt;
+
+    let mut ar = tar::Builder::new(Vec::new());
+
+    let mut header = tar::Header::new_gnu();
+    header.set_size(0);
+    header.set_entry_type(tar::EntryType::Directory);
+    t!(header.set_path("foo"));
+    header.set_mode(0o777);
+    header.set_cksum();
+    t!(ar.append(&header, &[][..]));
+
+    let bytes = t!(ar.into_inner());
+    let mut ar = tar::Archive::new(&bytes[..]);
+
+    let td = t!(TempDir::new("tar"));
+    t!(ar.unpack(td.path()));
+    let f = t!(File::open(td.path().join("foo")));
+    let md = t!(f.metadata());
+    assert!(md.is_dir());
+    assert_eq!(md.permissions().mode(), 0o40777);
+}
+
 #[test]
 fn modify_link_just_created() {
     let mut ar = tar::Builder::new(Vec::new());