fix: report legacy decodability and reuse format metadata

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- Legacy Office reports unencrypted documents as decodable and derives metadata
+  from the shared file-type table.
+
 - Legacy Excel rejects truncated BIFF records and a wrong substream type. It
   drops cells outside the BIFF8 grid, and numbers ignore the host locale.
 

src/odr/internal/oldms/oldms_file.cpp

@@ -1,6 +1,7 @@
 #include <odr/internal/oldms/oldms_file.hpp>
 
 #include <odr/exceptions.hpp>
+#include <odr/odr.hpp>
 
 #include <odr/internal/common/path.hpp>
 #include <odr/internal/oldms/presentation/ppt_document.hpp>
@@ -10,16 +11,16 @@
 #include <odr/internal/oldms/text/doc_document.hpp>
 #include <odr/internal/oldms/text/doc_parser.hpp>
 
+#include <array>
 #include <memory>
 #include <optional>
-#include <unordered_map>
+#include <utility>
 
 namespace odr::internal::oldms {
 
 namespace {
-/// Each format keeps the bytes that say so in the clear, so this is readable
-/// without the password. Detection only — odrcore cannot decrypt any of them.
-/// Nothing where the format's own probe could not read the signal.
+/// Probes the cleartext encryption marker; an unreadable marker leaves it
+/// unknown.
 std::optional<bool>
 parse_password_encrypted(const FileType type,
                          const abstract::ReadableFilesystem &files) {
@@ -36,37 +37,21 @@ parse_password_encrypted(const FileType type,
 }
 
 FileMeta parse_meta(const abstract::ReadableFilesystem &files) {
-  struct Variant {
-    FileType type{FileType::unknown};
-    DocumentType document_type{DocumentType::unknown};
-    std::string_view mimetype;
-  };
-
-  static const std::unordered_map<AbsPath, Variant> types = {
-      // MS-DOC: The "WordDocument" stream MUST be present in the file.
-      // https://msdn.microsoft.com/en-us/library/dd926131(v=office.12).aspx
-      {AbsPath("/WordDocument"),
-       {FileType::legacy_word_document, DocumentType::text,
-        "application/msword"}},
-      // MS-PPT: The "PowerPoint Document" stream MUST be present in the file.
-      // https://msdn.microsoft.com/en-us/library/dd911009(v=office.12).aspx
-      {AbsPath("/PowerPoint Document"),
-       {FileType::legacy_powerpoint_presentation, DocumentType::presentation,
-        "application/vnd.ms-powerpoint"}},
-      // MS-XLS: The "Workbook" stream MUST be present in the file.
-      // https://docs.microsoft.com/en-us/openspecs/office_file_formats/ms-ppt/1fc22d56-28f9-4818-bd45-67c2bf721ccf
-      {AbsPath("/Workbook"),
-       {FileType::legacy_excel_worksheets, DocumentType::spreadsheet,
-        "application/vnd.ms-excel"}},
+  // Each format MUST have its stream ([MS-DOC], [MS-PPT], [MS-XLS]).
+  static constexpr std::array types{
+      std::pair{"/WordDocument", FileType::legacy_word_document},
+      std::pair{"/PowerPoint Document",
+                FileType::legacy_powerpoint_presentation},
+      std::pair{"/Workbook", FileType::legacy_excel_worksheets},
   };
 
   FileMeta result;
 
-  for (const auto &[path, variant] : types) {
-    if (files.is_file(path)) {
-      result.type = variant.type;
-      result.mimetype = variant.mimetype;
-      result.document_type = variant.document_type;
+  for (const auto &[path, type] : types) {
+    if (files.is_file(AbsPath(path))) {
+      result.type = type;
+      result.mimetype = mimetype_by_file_type(type);
+      result.document_type = document_type_by_file_type(type);
       break;
     }
   }
@@ -84,9 +69,7 @@ LegacyMicrosoftFile::LegacyMicrosoftFile(
     : m_files{std::move(files)} {
   m_file_meta = parse_meta(*m_files);
 
-  // `EncryptionState::unknown` where the probe could not read the signal: a
-  // stream that cannot be inspected is not a document that said it is in the
-  // clear, and `FileMeta` has only the boolean to carry it.
+  // An unreadable encryption marker leaves the state unknown.
   const std::optional<bool> encrypted =
       parse_password_encrypted(m_file_meta.type, *m_files);
   m_file_meta.password_encrypted = encrypted.value_or(false);
@@ -127,11 +110,13 @@ std::shared_ptr<abstract::DecodedFile> LegacyMicrosoftFile::decrypt(
       "odrcore does not support decryption of legacy Microsoft files");
 }
 
-bool LegacyMicrosoftFile::is_decodable() const noexcept { return false; }
+bool LegacyMicrosoftFile::is_decodable() const noexcept {
+  return m_encryption_state != EncryptionState::encrypted;
+}
 
 std::shared_ptr<abstract::Document> LegacyMicrosoftFile::document() const {
-  // otherwise the encrypted bytes get read as structure, and the caller sees a
-  // parse error where a password prompt belongs
+  // The parser would read the ciphertext as structure, and the caller would see
+  // a parse error and not a password prompt.
   if (m_encryption_state == EncryptionState::encrypted) {
     throw FileEncryptedError();
   }

src/odr/internal/oldms/spreadsheet/AGENTS.md

@@ -76,8 +76,8 @@ returns the fill.
 - Fills (§2.5.20): `fls` 0 is none, solid (1) renders `icvFore`, every other
   pattern is approximated by its foreground color.
 - A `dyHeight` of 0 (allowed by §2.4.122) leaves `font_size` unset. The
-  `StyleRegistry` owns the parsed `Font` records, so `TextStyle::font_name`
-  (`const char *`) points into them and stays valid.
+  resolved `TextStyle` owns its font name; the raw Font records are discarded
+  after resolving the XF records.
 
 **Adapters** expose `ValueType::string` for every cell, `sheet_cell_span` of
 `{1,1}` and `sheet_cell_is_covered` false. Sheet, column and row styles are

src/odr/internal/oldms/spreadsheet/xls_parser.cpp

@@ -244,8 +244,7 @@ spreadsheet::parse_tree(ElementRegistry &registry,
   std::vector<std::string> shared_strings;
   GlobalStyles styles;
   parse_globals(reader, bound_sheets, shared_strings, styles);
-  style_registry =
-      StyleRegistry(std::move(styles.fonts), styles.xfs, styles.palette);
+  style_registry = StyleRegistry(styles.fonts, styles.xfs, styles.palette);
 
   auto [root_id, root] = registry.create_element(ElementType::root);
 

src/odr/internal/oldms/spreadsheet/xls_style.cpp

@@ -60,10 +60,9 @@ font_at(const std::span<const StyleRegistry::Font> fonts,
 
 } // namespace
 
-StyleRegistry::StyleRegistry(std::vector<Font> fonts,
+StyleRegistry::StyleRegistry(const std::span<const Font> fonts,
                              const std::span<const XfBody> xfs,
-                             const std::span<const LongRgb> palette)
-    : m_fonts{std::move(fonts)} {
+                             const std::span<const LongRgb> palette) {
   if (!palette.empty() && palette.size() != palette_color_count) {
     throw std::invalid_argument("StyleRegistry: unexpected palette size");
   }
@@ -73,7 +72,7 @@ StyleRegistry::StyleRegistry(std::vector<Font> fonts,
   for (const XfBody &xf : xfs) {
     ResolvedStyle &style = m_cell_styles.emplace_back();
 
-    const Font &font = font_at(m_fonts, xf.ifnt);
+    const Font &font = font_at(fonts, xf.ifnt);
     TextStyle &text = style.text_style;
     text.font_name = font.name;
     if (font.fixed.dyHeight != 0) {

src/odr/internal/oldms/spreadsheet/xls_style.hpp

@@ -23,17 +23,13 @@ class StyleRegistry final {
   StyleRegistry() = default;
   /// `palette` is the Palette record's `palette_color_count` colors, or empty
   /// when the record is absent (the spec's default palette applies).
-  StyleRegistry(std::vector<Font> fonts, std::span<const XfBody> xfs,
+  StyleRegistry(std::span<const Font> fonts, std::span<const XfBody> xfs,
                 std::span<const LongRgb> palette);
 
-  /// The resolved style of an XF record, by XF index (a cell's ixfe): the
-  /// text (font) side and the cell (fill) side. Throws if the index has no
-  /// XF record.
+  /// Font and cell style by XF index; throws for an unknown index.
   [[nodiscard]] const ResolvedStyle &cell_style(std::uint16_t ixfe) const;
 
 private:
-  /// The fonts the XF records index.
-  std::vector<Font> m_fonts;
   std::vector<ResolvedStyle> m_cell_styles;
 };
 

src/odr/internal/oldms/text/AGENTS.md

@@ -75,8 +75,7 @@ run's `Chpx.grpprl` applied over the default style.
 - Colors: `sprmCCv` is a `COLORREF` (`fAuto` leaves it unset). `sprmCIco` and
   `sprmCHighlight` use the `Ico` palette (§2.9.119). The spec's extracted
   table repeats `0x0C` for `0x0D`, which is dark red (`0x800000`).
-- Font names from `SttbfFfn` live in the `StyleRegistry` and never move, so
-  `TextStyle::font_name` (`const char *`) stays valid.
+- Resolved `TextStyle` values own their font names, copied from `SttbfFfn`.
 
 **`TextCleaner`.** `0x0D`, `0x0C` and `0x0B` never reach it, because the
 caller splits paragraphs, pages and lines on them. `0x13`, `0x14` and `0x15`

test/src/internal/oldms/encryption_test.cpp

@@ -91,6 +91,11 @@ std::string workbook_stream(const bool encrypted) {
 } // namespace
 
 TEST(OldMsEncryption, doc_reports_the_encrypted_flag) {
+  const LegacyMicrosoftFile clear(std::make_shared<VirtualFilesystem>(
+      filesystem_of("/WordDocument", word_document_stream(false))));
+  EXPECT_TRUE(clear.is_decodable());
+  EXPECT_EQ(clear.mimetype(), "application/msword");
+  EXPECT_EQ(clear.document_type(), DocumentType::text);
   EXPECT_EQ(text::password_encrypted(
                 filesystem_of("/WordDocument", word_document_stream(true))),
             true);
@@ -152,6 +157,7 @@ TEST(OldMsEncryption, an_unreadable_stream_leaves_the_state_unknown) {
   EXPECT_EQ(file.file_type(), FileType::legacy_word_document);
   EXPECT_FALSE(file.password_encrypted());
   EXPECT_EQ(file.encryption_state(), EncryptionState::unknown);
+  EXPECT_TRUE(file.is_decodable());
 }
 
 /// The file that reported the issue: it threw
@@ -162,6 +168,7 @@ TEST(OldMsEncryption, an_encrypted_doc_surfaces_as_encrypted) {
 
   EXPECT_EQ(file.file_type(), FileType::legacy_word_document);
   EXPECT_TRUE(file.password_encrypted());
+  EXPECT_FALSE(file.is_decodable());
   EXPECT_EQ(file.encryption_state(), EncryptionState::encrypted);
   EXPECT_THROW(static_cast<void>(file.as_document_file().document()),
                FileEncryptedError);

test/src/internal/oldms/ppt_test.cpp

@@ -131,6 +131,8 @@ TEST(OldMs, ppt_empty) {
   EXPECT_EQ(document_file.file_type(),
             FileType::legacy_powerpoint_presentation);
 
+  EXPECT_TRUE(document_file.is_decodable());
+
   const Document document = document_file.document();
   EXPECT_EQ(document.document_type(), DocumentType::presentation);
 

test/src/internal/oldms/xls_test.cpp

@@ -308,6 +308,8 @@ TEST(OldMs, xls_empty) {
 
   EXPECT_EQ(document_file.file_type(), FileType::legacy_excel_worksheets);
 
+  EXPECT_TRUE(document_file.is_decodable());
+
   const Document document = document_file.document();
   EXPECT_EQ(document.document_type(), DocumentType::spreadsheet);