Merge branch 'master' into fix/issues-41

.github/workflows/tests.yml

@@ -11,9 +11,23 @@ on:
       - '**.md'
 
 jobs:
+  tests:
+    strategy:
+      matrix:
+        os: [ ubuntu-latest, windows-latest, macos-latest]
+        rust: [ stable ]
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v3
+      - name: Install rust toolchain
+        uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ matrix.rust }}
+      - name: Test
+        run: cargo test --verbose
+
   linters:
     runs-on: ubuntu-latest
-
     steps:
       - uses: actions/checkout@v3
         with:
@@ -27,20 +41,3 @@ jobs:
         run: cargo fmt --all -- --check
       - name: Cargo clippy
         run: cargo clippy --all-features -- -D warnings
-
-  tests:
-    strategy:
-      matrix:
-        os: [ ubuntu-20.04, windows-2019, macos-11]
-        rust: [ stable ]
-
-    needs: [linters]
-    runs-on: ${{ matrix.os }}
-    steps:
-      - uses: actions/checkout@v3
-      - name: Install rust toolchain
-        uses: dtolnay/rust-toolchain@master
-        with:
-          toolchain: ${{ matrix.rust }}
-      - name: Test
-        run: cargo test --verbose

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cfb"
-version = "0.8.1"
+version = "0.10.0"
 authors = ["Matthew D. Steele <[email protected]>"]
 description = "Read/write Compound File Binary (structured storage) files"
 repository = "https://github.com/mdsteele/rust-cfb"
@@ -11,11 +11,11 @@ edition = "2018"
 
 [dependencies]
 byteorder = "1"
-fnv = "1.0.7"
+fnv = "1.0"
 uuid = "1"
 
 [dev-dependencies]
-clap = "2.27"
+clap = { version = "4.4", features = ["derive"] }
 rand = "0.8"
 rand_pcg = "0.3"
 time = "0.3"

examples/cfbtool.rs

@@ -1,9 +1,39 @@
-use clap::{App, Arg, SubCommand};
 use std::io;
 use std::path::PathBuf;
+
+use clap::{Parser, Subcommand};
 use time::OffsetDateTime;
 use uuid::Uuid;
 
+#[derive(Parser, Debug)]
+#[clap(author, about, long_about = None)]
+struct Cli {
+    #[clap(subcommand)]
+    command: Command,
+}
+
+#[derive(Subcommand, Debug)]
+enum Command {
+    /// Concatenates and prints streams
+    Cat { path: Vec<String> },
+
+    /// Changes storage CLSIDs
+    Chcls { clsid: Uuid, path: Vec<String> },
+
+    /// Lists storage contents
+    Ls {
+        #[clap(short, long)]
+        /// Lists in long format
+        long: bool,
+
+        #[clap(short, long)]
+        /// Includes . in output
+        all: bool,
+
+        path: Vec<String>,
+    },
+}
+
 fn split(path: &str) -> (PathBuf, PathBuf) {
     let mut pieces = path.splitn(2, ':');
     if let Some(piece1) = pieces.next() {
@@ -51,68 +81,33 @@ fn list_entry(name: &str, entry: &cfb::Entry, long: bool) {
 }
 
 fn main() {
-    let matches = App::new("cfbtool")
-        .version("0.1")
-        .author("Matthew D. Steele <[email protected]>")
-        .about("Inspects and modifies CFB files")
-        .subcommand(
-            SubCommand::with_name("cat")
-                .about("Concatenates and prints streams")
-                .arg(Arg::with_name("path").multiple(true)),
-        )
-        .subcommand(
-            SubCommand::with_name("chcls")
-                .about("Changes storage CLSIDs")
-                .arg(Arg::with_name("clsid").required(true))
-                .arg(Arg::with_name("path").multiple(true)),
-        )
-        .subcommand(
-            SubCommand::with_name("ls")
-                .about("Lists storage contents")
-                .arg(
-                    Arg::with_name("all")
-                        .short("a")
-                        .help("Includes . in output"),
-                )
-                .arg(
-                    Arg::with_name("long")
-                        .short("l")
-                        .help("Lists in long format"),
-                )
-                .arg(Arg::with_name("path").multiple(true)),
-        )
-        .get_matches();
-    if let Some(submatches) = matches.subcommand_matches("cat") {
-        if let Some(paths) = submatches.values_of("path") {
-            for path in paths {
-                let (comp_path, inner_path) = split(path);
+    let cli = Cli::parse();
+    match cli.command {
+        Command::Cat { path } => {
+            for path in path {
+                let (comp_path, inner_path) = split(&path);
                 let mut comp = cfb::open(&comp_path).unwrap();
                 let mut stream = comp.open_stream(inner_path).unwrap();
                 io::copy(&mut stream, &mut io::stdout()).unwrap();
             }
         }
-    } else if let Some(submatches) = matches.subcommand_matches("chcls") {
-        let clsid =
-            Uuid::parse_str(submatches.value_of("clsid").unwrap()).unwrap();
-        if let Some(paths) = submatches.values_of("path") {
-            for path in paths {
-                let (comp_path, inner_path) = split(path);
+        Command::Chcls { clsid, path } => {
+            for path in path {
+                let (comp_path, inner_path) = split(&path);
                 let mut comp = cfb::open(&comp_path).unwrap();
                 comp.set_storage_clsid(inner_path, clsid).unwrap();
                 comp.flush().unwrap();
             }
         }
-    } else if let Some(submatches) = matches.subcommand_matches("ls") {
-        if let Some(paths) = submatches.values_of("path") {
-            let long = submatches.is_present("long");
-            for path in paths {
-                let (comp_path, inner_path) = split(path);
+        Command::Ls { long, all, path } => {
+            for path in path {
+                let (comp_path, inner_path) = split(&path);
                 let comp = cfb::open(&comp_path).unwrap();
                 let entry = comp.entry(&inner_path).unwrap();
                 if entry.is_stream() {
                     list_entry(entry.name(), &entry, long);
                 } else {
-                    if submatches.is_present("all") {
+                    if all {
                         list_entry(".", &entry, long);
                     }
                     for subentry in comp.read_storage(&inner_path).unwrap() {

src/internal/directory.rs

@@ -413,13 +413,40 @@ impl<F: Write + Seek> Directory<F> {
         if self.dir_entries.len() % dir_entries_per_sector == 0 {
             let start_sector = self.dir_start_sector;
             self.allocator.extend_chain(start_sector, SectorInit::Dir)?;
+            self.update_num_dir_sectors()?;
         }
         // Add a new entry to the end of the directory and return it.
         let stream_id = self.dir_entries.len() as u32;
         self.dir_entries.push(unallocated_dir_entry);
         Ok(stream_id)
     }
 
+    /// Increase header num_dir_sectors if version V4
+    /// note: not updating this value breaks ole32 compatibility
+    fn update_num_dir_sectors(&mut self) -> io::Result<()> {
+        let start_sector = self.dir_start_sector;
+        if self.version() == Version::V4 {
+            let num_dir_sectors =
+                self.count_directory_sectors(start_sector)?;
+            self.seek_within_header(40)?
+                .write_u32::<LittleEndian>(num_dir_sectors)?;
+        }
+        Ok(())
+    }
+
+    fn count_directory_sectors(
+        &mut self,
+        start_sector: u32,
+    ) -> io::Result<u32> {
+        let mut num_dir_sectors = 1;
+        let mut next_sector = self.allocator.next(start_sector)?;
+        while next_sector != consts::END_OF_CHAIN {
+            num_dir_sectors += 1;
+            next_sector = self.allocator.next(next_sector)?;
+        }
+        Ok(num_dir_sectors)
+    }
+
     /// Deallocates the specified directory entry.
     fn free_dir_entry(&mut self, stream_id: u32) -> io::Result<()> {
         debug_assert_ne!(stream_id, consts::ROOT_STREAM_ID);
@@ -428,6 +455,7 @@ impl<F: Write + Seek> Directory<F> {
         *self.dir_entry_mut(stream_id) = dir_entry;
         // TODO: Truncate directory chain if last directory sector is now all
         //       unallocated.
+        //       In that case, also call update_num_dir_sectors()
         Ok(())
     }
 

src/internal/entry.rs

@@ -1,8 +1,7 @@
 use crate::internal::{consts, DirEntry, MiniAllocator, ObjType, Timestamp};
-use std::cell::RefCell;
 use std::fmt;
 use std::path::{Path, PathBuf};
-use std::rc::Rc;
+use std::sync::{Arc, RwLock};
 use std::time::SystemTime;
 use uuid::Uuid;
 
@@ -125,14 +124,14 @@ pub struct Entries<'a, F: 'a> {
     // a reference to the Rc.  That would allow e.g. opening streams during
     // iteration.  But we'd need to think about how the iterator should behave
     // if the CFB tree structure is modified during iteration.
-    minialloc: &'a Rc<RefCell<MiniAllocator<F>>>,
+    minialloc: &'a Arc<RwLock<MiniAllocator<F>>>,
     stack: Vec<(PathBuf, u32, bool)>,
 }
 
 impl<'a, F> Entries<'a, F> {
     pub(crate) fn new(
         order: EntriesOrder,
-        minialloc: &'a Rc<RefCell<MiniAllocator<F>>>,
+        minialloc: &'a Arc<RwLock<MiniAllocator<F>>>,
         parent_path: PathBuf,
         start: u32,
     ) -> Entries<'a, F> {
@@ -149,7 +148,7 @@ impl<'a, F> Entries<'a, F> {
     }
 
     fn stack_left_spine(&mut self, parent_path: &Path, mut current_id: u32) {
-        let minialloc = self.minialloc.borrow();
+        let minialloc = self.minialloc.read().unwrap();
         while current_id != consts::NO_STREAM {
             self.stack.push((parent_path.to_path_buf(), current_id, true));
             current_id = minialloc.dir_entry(current_id).left_sibling;
@@ -162,7 +161,7 @@ impl<'a, F> Iterator for Entries<'a, F> {
 
     fn next(&mut self) -> Option<Entry> {
         if let Some((parent, stream_id, visit_siblings)) = self.stack.pop() {
-            let minialloc = self.minialloc.borrow();
+            let minialloc = self.minialloc.read().unwrap();
             let dir_entry = minialloc.dir_entry(stream_id);
             let path = join_path(&parent, dir_entry);
             if visit_siblings {
@@ -201,9 +200,8 @@ mod tests {
         Allocator, DirEntry, Directory, MiniAllocator, ObjType, Sectors,
         Timestamp, Validation, Version,
     };
-    use std::cell::RefCell;
     use std::path::{Path, PathBuf};
-    use std::rc::Rc;
+    use std::sync::{Arc, RwLock};
 
     fn make_entry(
         name: &str,
@@ -219,7 +217,7 @@ mod tests {
         dir_entry
     }
 
-    fn make_minialloc() -> Rc<RefCell<MiniAllocator<()>>> {
+    fn make_minialloc() -> Arc<RwLock<MiniAllocator<()>>> {
         // Root contains:      3 contains:
         //      5                  8
         //     / \                / \
@@ -261,7 +259,7 @@ mod tests {
             Validation::Strict,
         )
         .unwrap();
-        Rc::new(RefCell::new(minialloc))
+        Arc::new(RwLock::new(minialloc))
     }
 
     fn paths_for_entries(entries: &[Entry]) -> Vec<&Path> {