fix: retain CFB stream metadata and traversal context

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- CFB streams retain their entry metadata after the file wrapper is released.
+  Nested directory walks finish children before resuming outer siblings.
+
 - JSON edits reject fractional and out-of-range indices before changing a
   document, and preserve the full width of positive element identifiers.
   Text and document edits validate the version and operations array.

src/odr/internal/cfb/cfb_impl.cpp

@@ -47,15 +47,15 @@ std::string CompoundFileEntry::get_name() const {
   if (name_len < 2) {
     return {};
   }
-  return internal::util::string::c16str_to_string(name, name_len - 2);
+  return internal::util::string::c16str_to_string(name.data(), name_len - 2);
 }
 
 CompoundFileReader::CompoundFileReader(std::istream &in,
                                        const std::uint64_t file_size)
     : m_file_size{file_size} {
   parse_header(in, m_header);
 
-  if (std::memcmp(m_header.signature, MAGIC, 8) != 0) {
+  if (std::memcmp(m_header.signature.data(), MAGIC, 8) != 0) {
     throw NoCfbFile();
   }
 
@@ -80,7 +80,8 @@ CompoundFileReader::CompoundFileReader(std::istream &in,
 void CompoundFileReader::parse_entry(std::istream &in,
                                      const std::uint32_t entry_id,
                                      CompoundFileEntry &entry) const {
-  const std::uint64_t offset = entry_id * sizeof(CompoundFileEntry);
+  const std::uint64_t offset =
+      std::uint64_t{entry_id} * sizeof(CompoundFileEntry);
 
   if (offset >= m_file_size) {
     throw std::invalid_argument("");
@@ -129,9 +130,6 @@ void CompoundFileReader::read_stream(std::istream &in,
   SectorOffset current_sector_offset =
       normalize_sector_offset(in, sector_offset);
 
-  // copy as many as possible in each step
-  // copy_length typically iterate as: m_sectorSize - offset   --> m_sectorSize
-  // -->   m_sectorSize  --> ... -->    remaining
   while (length > 0) {
     const std::uint64_t address =
         sector_offset_to_address(current_sector_offset);
@@ -142,10 +140,13 @@ void CompoundFileReader::read_stream(std::istream &in,
     }
 
     in.seekg(static_cast<std::streampos>(address));
-    in.read(buffer, static_cast<std::streamsize>(copy_length));
+    util::byte_stream::read(in, buffer, copy_length);
     buffer += copy_length;
     length -= copy_length;
 
+    if (length == 0) {
+      break;
+    }
     current_sector_offset.sector =
         resolve_next_sector(in, current_sector_offset.sector);
     current_sector_offset.offset = 0;
@@ -159,9 +160,6 @@ void CompoundFileReader::read_mini_stream(std::istream &in,
   SectorOffset current_sector_offset =
       normalize_mini_sector_offset(in, sector_offset);
 
-  // copy as many as possible in each step
-  // copy_length typically iterate as: m_sectorSize - offset   --> m_sectorSize
-  // -->   m_sectorSize  --> ... -->    remaining
   while (length > 0) {
     const std::uint64_t address =
         mini_sector_offset_to_address(in, current_sector_offset);
@@ -172,10 +170,13 @@ void CompoundFileReader::read_mini_stream(std::istream &in,
     }
 
     in.seekg(static_cast<std::streampos>(address));
-    in.read(buffer, static_cast<std::streamsize>(copy_length));
+    util::byte_stream::read(in, buffer, copy_length);
     buffer += copy_length;
     length -= copy_length;
 
+    if (length == 0) {
+      break;
+    }
     current_sector_offset.sector =
         resolve_next_mini_sector(in, current_sector_offset.sector);
     current_sector_offset.offset = 0;

src/odr/internal/cfb/cfb_impl.hpp

@@ -2,6 +2,7 @@
 
 #include <odr/internal/abstract/file.hpp>
 
+#include <array>
 #include <cstdint>
 #include <string>
 
@@ -15,14 +16,14 @@ static constexpr std::uint32_t RootId = 0;
 #pragma pack(push, 1)
 
 struct CompoundFileHeader {
-  std::uint8_t signature[8];
-  std::uint8_t unused_clsid[16];
+  std::array<std::uint8_t, 8> signature;
+  std::array<std::uint8_t, 16> unused_clsid;
   std::uint16_t minor_version;
   std::uint16_t major_version;
   std::uint16_t byte_order;
   std::uint16_t sector_shift;
   std::uint16_t mini_sector_shift;
-  std::uint8_t reserved[6];
+  std::array<std::uint8_t, 6> reserved;
   std::uint32_t num_directory_sector;
   std::uint32_t num_fat_sector;
   std::uint32_t first_directory_sector_location;
@@ -32,18 +33,18 @@ struct CompoundFileHeader {
   std::uint32_t num_mini_fat_sector;
   std::uint32_t first_difat_sector_location;
   std::uint32_t num_difat_sector;
-  std::uint32_t header_difat[109];
+  std::array<std::uint32_t, 109> header_difat;
 };
 
 struct CompoundFileEntry {
-  char16_t name[32];
+  std::array<char16_t, 32> name;
   std::uint16_t name_len;
   std::uint8_t type;
   std::uint8_t color_flag;
   std::uint32_t left_sibling_id;
   std::uint32_t right_sibling_id;
   std::uint32_t child_id;
-  std::uint8_t clsid[16];
+  std::array<std::uint8_t, 16> clsid;
   std::uint32_t state_bits;
   std::uint64_t creation_time;
   std::uint64_t modified_time;
@@ -61,6 +62,9 @@ struct CompoundFileEntry {
 
 #pragma pack(pop)
 
+static_assert(sizeof(CompoundFileHeader) == 512);
+static_assert(sizeof(CompoundFileEntry) == 128);
+
 void parse_header(std::istream &in, CompoundFileHeader &hdr);
 void parse_entry(std::istream &in, CompoundFileEntry &entry);
 CompoundFileEntry parse_entry(std::istream &in);
@@ -79,19 +83,14 @@ class CompoundFileReader final {
     return m_root;
   }
 
-  /// Get entry (directory or file) by its ID.
-  /// Pass "0" to get the root directory entry. -- This is the start point to
-  /// navigate the compound file. Use the returned object to access child
-  /// entries.
+  /// Reads a directory entry by ID; zero names the root ([MS-CFB] 2.6.1).
   [[nodiscard]] CompoundFileEntry parse_entry(std::istream &in,
                                               std::uint32_t entry_id) const;
 
   void parse_entry(std::istream &in, std::uint32_t entry_id,
                    CompoundFileEntry &entry) const;
 
-  /// Get file(stream) data start with "offset".
-  /// The buffer must have enough space to store "len" bytes. Typically, "len"
-  /// is derived by the steam length.
+  /// Reads @p len bytes at @p offset into a buffer of at least @p len bytes.
   void read_file(std::istream &in, const CompoundFileEntry &entry,
                  std::uint64_t offset, char *buffer, std::uint64_t len) const;
 
@@ -106,7 +105,7 @@ class CompoundFileReader final {
   void read_stream(std::istream &in, const SectorOffset &sector_offset,
                    char *buffer, std::uint64_t length) const;
 
-  // Same logic as "ReadStream" except that use MiniStream functions instead
+  /// Reads a stream stored in mini sectors ([MS-CFB] 2.4).
   void read_mini_stream(std::istream &in, const SectorOffset &sector_offset,
                         char *buffer, std::uint64_t length) const;
 
@@ -124,8 +123,7 @@ class CompoundFileReader final {
   mini_sector_offset_to_address(std::istream &in,
                                 const SectorOffset &sector_offset) const;
 
-  /// Locate the final sector/offset when original offset expands multiple
-  /// sectors
+  /// Follows the sector chain to the requested offset.
   [[nodiscard]] SectorOffset
   normalize_sector_offset(std::istream &in, SectorOffset sector_offset) const;
 

src/odr/internal/cfb/cfb_util.cpp

@@ -21,21 +21,21 @@ class ReaderBuffer final : public std::streambuf {
                const impl::CompoundFileEntry &entry,
                std::unique_ptr<std::istream> stream,
                const std::size_t buffer_size = 4096)
-      : m_reader{&reader}, m_entry{&entry}, m_stream{std::move(stream)},
+      : m_reader{&reader}, m_entry{entry}, m_stream{std::move(stream)},
         m_buffer(buffer_size, '\0') {
     setg(m_buffer.data(), m_buffer.data(), m_buffer.data());
   }
 
 protected:
   int underflow() override {
-    if (m_offset >= m_entry->size) {
+    if (m_offset >= m_entry.size) {
       return traits_type::eof();
     }
 
-    const std::uint64_t remaining = m_entry->size - m_offset;
+    const std::uint64_t remaining = m_entry.size - m_offset;
     const std::uint64_t amount =
         std::min<std::uint64_t>(remaining, m_buffer.size());
-    m_reader->read_file(*m_stream, *m_entry, m_offset, m_buffer.data(), amount);
+    m_reader->read_file(*m_stream, m_entry, m_offset, m_buffer.data(), amount);
     m_offset += amount;
     setg(m_buffer.data(), m_buffer.data(), m_buffer.data() + amount);
 
@@ -54,11 +54,11 @@ class ReaderBuffer final : public std::streambuf {
       return -1;
     }
 
-    if (m_entry->size > static_cast<std::uint64_t>(
-                            std::numeric_limits<std::streamoff>::max())) {
+    if (m_entry.size > static_cast<std::uint64_t>(
+                           std::numeric_limits<std::streamoff>::max())) {
       return -1;
     }
-    const auto size = static_cast<std::streamoff>(m_entry->size);
+    const auto size = static_cast<std::streamoff>(m_entry.size);
     std::streamoff base = 0;
     if (dir == std::ios_base::cur) {
       base = static_cast<std::streamoff>(m_offset) - (egptr() - gptr());
@@ -81,7 +81,7 @@ class ReaderBuffer final : public std::streambuf {
 
 private:
   const impl::CompoundFileReader *m_reader{};
-  const impl::CompoundFileEntry *m_entry{};
+  const impl::CompoundFileEntry m_entry;
   std::unique_ptr<std::istream> m_stream;
   std::uint64_t m_offset{0};
   std::vector<char> m_buffer;
@@ -205,7 +205,7 @@ void Archive::Iterator::next_() {
   }
 
   if (const std::optional<Entry> child = m_entry->child(); child.has_value()) {
-    m_directories.push_back(*m_entry);
+    m_directories.push_back({*m_entry, m_ancestors.size()});
     enter_(*child);
     dig_left_();
     return;
@@ -215,30 +215,29 @@ void Archive::Iterator::next_() {
 }
 
 void Archive::Iterator::next_flat_() {
-  if (!m_entry.has_value()) {
-    return;
-  }
-
-  if (const std::optional<Entry> right = m_entry->right(); right.has_value()) {
-    enter_(*right);
-    dig_left_();
-    return;
-  }
+  while (m_entry.has_value()) {
+    if (const std::optional<Entry> right = m_entry->right();
+        right.has_value()) {
+      enter_(*right);
+      dig_left_();
+      return;
+    }
 
-  if (!m_ancestors.empty()) {
-    m_entry = m_ancestors.back();
-    m_ancestors.pop_back();
-    return;
-  }
+    if (!m_ancestors.empty() &&
+        (m_directories.empty() ||
+         m_ancestors.size() > m_directories.back().ancestors)) {
+      m_entry = m_ancestors.back();
+      m_ancestors.pop_back();
+      return;
+    }
 
-  if (!m_directories.empty()) {
-    m_entry = m_directories.back();
+    if (m_directories.empty()) {
+      m_entry.reset();
+      return;
+    }
+    m_entry = m_directories.back().entry;
     m_directories.pop_back();
-    next_flat_();
-    return;
   }
-
-  m_entry = {};
 }
 
 const impl::CompoundFileReader &Archive::cfb() const { return m_cfb; }

src/odr/internal/cfb/cfb_util.hpp

@@ -47,7 +47,7 @@ class Archive final : public std::enable_shared_from_this<Archive> {
           m_path{std::move(path)} {}
 
     bool operator==(const Entry &other) const {
-      return m_entry_id == other.m_entry_id;
+      return m_archive == other.m_archive && m_entry_id == other.m_entry_id;
     }
 
     [[nodiscard]] bool is_file() const { return m_entry.is_file(); }
@@ -103,9 +103,13 @@ class Archive final : public std::enable_shared_from_this<Archive> {
     }
 
   private:
+    struct Directory final {
+      Entry entry;
+      std::size_t ancestors;
+    };
     std::optional<Entry> m_entry;
     std::vector<Entry> m_ancestors;
-    std::vector<Entry> m_directories;
+    std::vector<Directory> m_directories;
     std::set<std::uint32_t> m_visited;
 
     Iterator() = default;