fix: skip data reads on directory entries for RAR archives

libarchive's RAR reader returns ARCHIVE_FAILED on `archive_read_data_block`
for directory entries, causing every RAR containing directories to error
with "Can't decompress an entry marked as a directory". Skip the data-read
step for directory entries in `uncompress_archive`, `uncompress_archive_file`,
and `ArchiveIterator`.

Closes #131

Author: otavio

CHANGES.md

@@ -4,6 +4,10 @@
 
 ## [Unreleased] - ReleaseDate
 
+* Fix RAR extraction failing with "Can't decompress an entry marked as a
+  directory" whenever the archive contains directory entries. Data reads are
+  now skipped for directory entries in `uncompress_archive`,
+  `uncompress_archive_file`, and `ArchiveIterator` [#131]
 * Add `list_archive_entries` (and `_with_encoding` variant) returning path and
   uncompressed size per entry, so callers can obtain per-file sizes without
   extracting the archive. Mirrored in `async_support`, `futures_support`, and

src/iterator.rs

@@ -12,8 +12,8 @@ use crate::stat;
 use libc::stat;
 
 use crate::{
-    error::archive_result, ffi, ffi::UTF8LocaleGuard, DecodeCallback, Error, Result,
-    READER_BUFFER_SIZE,
+    error::archive_result, ffi, ffi::UTF8LocaleGuard, libarchive_entry_is_dir, DecodeCallback,
+    Error, Result, READER_BUFFER_SIZE,
 };
 
 struct HeapReadSeekerPipe<R: Read + Seek> {
@@ -76,6 +76,7 @@ pub struct ArchiveIterator<R: Read + Seek> {
 
     decode: DecodeCallback,
     in_file: bool,
+    current_is_dir: bool,
     closed: bool,
     error: bool,
     filter: Option<Box<EntryFilterCallbackFn>>,
@@ -243,6 +244,7 @@ impl<R: Read + Seek> ArchiveIterator<R> {
 
                 decode,
                 in_file: false,
+                current_is_dir: false,
                 closed: false,
                 error: false,
                 filter,
@@ -380,13 +382,18 @@ impl<R: Read + Seek> ArchiveIterator<R> {
                     Err(e) => return ArchiveContents::Err(e),
                 };
                 let stat = *ffi::archive_entry_stat(self.archive_entry);
+                self.current_is_dir = libarchive_entry_is_dir(self.archive_entry);
                 ArchiveContents::StartOfEntry(file_name, stat)
             }
             _ => ArchiveContents::Err(Error::from(self.archive_reader)),
         }
     }
 
     unsafe fn next_data_chunk(&mut self) -> ArchiveContents {
+        if self.current_is_dir {
+            return ArchiveContents::EndOfEntry;
+        }
+
         let mut buffer = std::ptr::null();
         let mut offset = 0;
         let mut size = 0;

src/lib.rs

@@ -380,7 +380,9 @@ where
                     ffi::archive_write_header(archive_writer, entry),
                     archive_writer,
                 )?;
-                libarchive_copy_data(archive_reader, archive_writer)?;
+                if !libarchive_entry_is_dir(entry) {
+                    libarchive_copy_data(archive_reader, archive_writer)?;
+                }
 
                 archive_result_strict(
                     ffi::archive_write_finish_entry(archive_writer),
@@ -470,6 +472,9 @@ where
                 }
             }
 
+            if libarchive_entry_is_dir(entry) {
+                return Ok(0);
+            }
             libarchive_write_data_block(archive_reader, target)
         },
     )
@@ -701,6 +706,15 @@ fn libarchive_entry_size(entry: *mut ffi::archive_entry) -> u64 {
     size.max(0) as u64
 }
 
+// Raw POSIX mode bits: `libc::S_IFDIR` is not exposed on Windows, where our
+// `stat` mirrors libarchive's own layout.
+pub(crate) fn libarchive_entry_is_dir(entry: *mut ffi::archive_entry) -> bool {
+    const S_IFMT: u32 = 0o170000;
+    const S_IFDIR: u32 = 0o040000;
+    let mode = unsafe { (*ffi::archive_entry_stat(entry)).st_mode } as u32;
+    (mode & S_IFMT) == S_IFDIR
+}
+
 fn libarchive_entry_pathname<'a>(entry: *mut ffi::archive_entry) -> Result<&'a CStr> {
     let pathname = unsafe { ffi::archive_entry_pathname(entry) };
     if pathname.is_null() {

tests/fixtures/tree.rar

@@ -556,6 +556,82 @@ fn uncompress_7z_to_dir_not_preserve_owner() {
     );
 }
 
+#[test]
+fn get_a_file_from_rar() {
+    let mut source = std::fs::File::open("tests/fixtures/tree.rar").unwrap();
+    let mut target = Vec::default();
+
+    let written = uncompress_archive_file(&mut source, &mut target, "tree/branch2/leaf")
+        .expect("Failed to get the file");
+    assert_eq!(
+        String::from_utf8_lossy(&target),
+        "Goodbye World\n",
+        "Uncompressed file did not match",
+    );
+    assert_eq!(written, 14, "Uncompressed bytes count did not match");
+}
+
+#[test]
+fn iterate_rar_entries() {
+    let source = std::fs::File::open("tests/fixtures/tree.rar").unwrap();
+
+    let mut names = Vec::new();
+    let mut content = Vec::new();
+
+    for item in ArchiveIterator::from_read(source).expect("Failed to read archive") {
+        match item {
+            ArchiveContents::StartOfEntry(name, _) => names.push(name),
+            ArchiveContents::DataChunk(chunk) => {
+                if names
+                    .last()
+                    .map(|n| n == "tree/branch2/leaf")
+                    .unwrap_or(false)
+                {
+                    content.extend_from_slice(&chunk);
+                }
+            }
+            ArchiveContents::EndOfEntry => {}
+            ArchiveContents::Err(e) => panic!("iterator errored: {}", e),
+        }
+    }
+
+    assert!(
+        names.iter().any(|n| n == "tree/branch1"),
+        "directory entry missing from iteration: {:?}",
+        names
+    );
+    assert!(
+        names.iter().any(|n| n == "tree/branch2/leaf"),
+        "tree/branch2/leaf missing from iteration: {:?}",
+        names
+    );
+    assert_eq!(String::from_utf8_lossy(&content), "Goodbye World\n");
+}
+
+#[test]
+fn uncompress_rar_to_dir() {
+    let dir = tempfile::TempDir::new().expect("Failed to create the tmp directory");
+    let mut source = std::fs::File::open("tests/fixtures/tree.rar").unwrap();
+
+    uncompress_archive(&mut source, dir.path(), Ownership::Ignore)
+        .expect("Failed to uncompress the file");
+
+    assert!(
+        dir.path().join("tree/branch1/leaf").exists(),
+        "the path doesn't exist"
+    );
+    assert!(
+        dir.path().join("tree/branch2/leaf").exists(),
+        "the path doesn't exist"
+    );
+
+    let contents = std::fs::read_to_string(dir.path().join("tree/branch2/leaf")).unwrap();
+    assert_eq!(
+        contents, "Goodbye World\n",
+        "Uncompressed file did not match"
+    );
+}
+
 #[test]
 fn uncompress_to_dir_with_utf8_pathname() {
     let dir = tempfile::TempDir::new().expect("Failed to create the tmp directory");