fix(open): honour the requested type for every document container

CHANGELOG.md

@@ -16,6 +16,12 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- `open(file, as)` honours `as` for a packaged OpenDocument, an ooxml and a
+  legacy MS document the way it already did for a flat OpenDocument: a
+  container that turns out to hold another document type is no longer returned
+  as the type that was asked for. An encrypted ooxml is unchanged - it names no
+  inner type until it is decrypted, and still opens as whichever was asked for.
+
 - Flat OpenDocument files decode as documents rather than as an xml source
   view: `.fodt`, `.fodp`, `.fods` and `.fodg` open, render, edit and save like
   their packaged counterparts. Their images ride in the markup, and an

src/odr/internal/open_strategy.cpp

@@ -47,6 +47,14 @@ template <typename T> auto priority_comparator(const std::vector<T> &priority) {
   };
 }
 
+/// Whether @p file is the ooxml that was asked for. An encrypted one names no
+/// inner type until it is decrypted, so it answers for whichever was asked.
+bool is_the_requested_ooxml(const ooxml::OfficeOpenXmlFile &file,
+                            const FileType as) {
+  return file.file_type() == as ||
+         file.file_type() == FileType::office_open_xml_encrypted;
+}
+
 /// Decodes @p file as exactly @p as, or throws the format's "not a ..."
 /// exception (@ref UnsupportedFileType for a type we cannot decode at all).
 std::unique_ptr<abstract::DecodedFile>
@@ -60,7 +68,11 @@ open_file_as(const std::shared_ptr<abstract::File> &file, const FileType as,
     try {
       auto zip_file = std::make_unique<zip::ZipFile>(file);
       auto filesystem = zip_file->archive()->as_filesystem();
-      return std::make_unique<odf::OpenDocumentFile>(filesystem);
+      auto odf_file = std::make_unique<odf::OpenDocumentFile>(filesystem);
+      if (odf_file->file_type() == as) {
+        return odf_file;
+      }
+      ODR_VERBOSE(logger, "odf is a different document type");
     } catch (...) {
       ODR_VERBOSE(logger, "failed to open as odf");
     }
@@ -87,14 +99,22 @@ open_file_as(const std::shared_ptr<abstract::File> &file, const FileType as,
     try {
       auto zip_file = std::make_unique<zip::ZipFile>(file);
       auto filesystem = zip_file->archive()->as_filesystem();
-      return std::make_unique<ooxml::OfficeOpenXmlFile>(filesystem);
+      auto ooxml_file = std::make_unique<ooxml::OfficeOpenXmlFile>(filesystem);
+      if (is_the_requested_ooxml(*ooxml_file, as)) {
+        return ooxml_file;
+      }
+      ODR_VERBOSE(logger, "ooxml zip is a different document type");
     } catch (...) {
       ODR_VERBOSE(logger, "failed to open as ooxml zip");
     }
     try {
       auto cfb_file = std::make_unique<cfb::CfbFile>(file);
       auto filesystem = cfb_file->archive()->as_filesystem();
-      return std::make_unique<ooxml::OfficeOpenXmlFile>(filesystem);
+      auto ooxml_file = std::make_unique<ooxml::OfficeOpenXmlFile>(filesystem);
+      if (is_the_requested_ooxml(*ooxml_file, as)) {
+        return ooxml_file;
+      }
+      ODR_VERBOSE(logger, "ooxml cfb is a different document type");
     } catch (...) {
       ODR_VERBOSE(logger, "failed to open as ooxml cfb");
     }
@@ -108,7 +128,12 @@ open_file_as(const std::shared_ptr<abstract::File> &file, const FileType as,
     try {
       auto cfb_file = std::make_unique<cfb::CfbFile>(file);
       auto filesystem = cfb_file->archive()->as_filesystem();
-      return std::make_unique<oldms::LegacyMicrosoftFile>(filesystem);
+      auto oldms_file =
+          std::make_unique<oldms::LegacyMicrosoftFile>(filesystem);
+      if (oldms_file->file_type() == as) {
+        return oldms_file;
+      }
+      ODR_VERBOSE(logger, "legacy ms is a different document type");
     } catch (...) {
       ODR_VERBOSE(logger, "failed to open as legacy ms");
     }

test/src/file_test.cpp

@@ -28,6 +28,62 @@ TEST(File, from_disk_matches_the_path_constructor) {
   EXPECT_EQ(file.size(), File(path).size());
 }
 
+/// `open(file, as)` decodes as exactly what it is asked for. A container
+/// names its own document type, and `as` is a claim about what is inside it -
+/// so a claim the container contradicts is no reading of the file at all.
+TEST(File, opening_as_the_wrong_document_type_throws) {
+  const struct {
+    const char *path;
+    FileType is;
+    FileType is_not;
+  } cases[]{
+      {"odr-public/odt/about.odt", FileType::opendocument_text,
+       FileType::opendocument_graphics},
+      {"odr-public/docx/file-sample_100kB.docx",
+       FileType::office_open_xml_document,
+       FileType::office_open_xml_presentation},
+      {"odr-public/doc/file-sample_100kB.doc", FileType::legacy_word_document,
+       FileType::legacy_excel_worksheets},
+  };
+
+  for (const auto &[path, is, is_not] : cases) {
+    const std::string file_path = TestData::test_file_path(path);
+
+    EXPECT_EQ(DecodedFile(file_path, is).file_type(), is) << path;
+    EXPECT_THROW(std::ignore = DecodedFile(file_path, is_not), UnknownFileType)
+        << path;
+  }
+}
+
+/// The one reading that is not its own container's: an encrypted ooxml names
+/// no inner type until it is decrypted, so it stands in for the one asked for.
+TEST(File, an_encrypted_ooxml_opens_as_the_type_asked_for) {
+  const DecodedFile file(
+      TestData::test_file_path("odr-public/docx/encrypted.docx"),
+      FileType::office_open_xml_document);
+
+  EXPECT_EQ(file.file_type(), FileType::office_open_xml_encrypted);
+  EXPECT_TRUE(file.password_encrypted());
+}
+
+/// The same claim about the same document in its other encoding answers the
+/// same way - which is what this fix is about.
+TEST(File, a_flat_document_and_a_package_answer_a_wrong_type_alike) {
+  const std::string flat =
+      R"(<?xml version="1.0" encoding="UTF-8"?>)"
+      R"(<office:document office:mimetype=")"
+      R"(application/vnd.oasis.opendocument.text">)"
+      R"(<office:body><office:text/></office:body></office:document>)";
+
+  EXPECT_THROW(std::ignore = DecodedFile(File::from_memory(flat),
+                                         FileType::opendocument_graphics),
+               UnknownFileType);
+  EXPECT_THROW(std::ignore = DecodedFile(
+                   TestData::test_file_path("odr-public/odt/about.odt"),
+                   FileType::opendocument_graphics),
+               UnknownFileType);
+}
+
 TEST(File, from_memory_holds_its_bytes) {
   const File file = File::from_memory("hello");