diff --git a/CHANGELOG.md b/CHANGELOG.md index b0cb1144c..803e6d60b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,12 @@ The release run heads these entries with the version and opens a fresh no run, as a new odt or docx renders. Typing there went nowhere and said nothing. +### API, files and saving + +- `DecodedFile::file()` returns the file an odf, ooxml, iWork or legacy Office + package was read from. It returned an empty handle, so `file().name()` threw + `NullPointerError`, and so did `fileName` in npm for every such document. + ## v7.5.1 - 2026-10-10 ### Spreadsheets diff --git a/src/odr/internal/iwork/iwork_file.cpp b/src/odr/internal/iwork/iwork_file.cpp index 159608d84..21e27b337 100644 --- a/src/odr/internal/iwork/iwork_file.cpp +++ b/src/odr/internal/iwork/iwork_file.cpp @@ -59,8 +59,9 @@ FileType parse_file_type(const abstract::ReadableFilesystem &filesystem) { } // namespace -IworkFile::IworkFile(std::shared_ptr filesystem) - : m_filesystem{std::move(filesystem)} { +IworkFile::IworkFile(std::shared_ptr filesystem, + std::shared_ptr source) + : m_filesystem{std::move(filesystem)}, m_source{std::move(source)} { if (!m_filesystem->is_file(AbsPath("/Index/Document.iwa"))) { throw NoIworkFile(); } @@ -70,7 +71,9 @@ IworkFile::IworkFile(std::shared_ptr filesystem) m_file_meta.document_type = document_type_by_file_type(m_file_meta.type); } -std::shared_ptr IworkFile::file() const noexcept { return {}; } +std::shared_ptr IworkFile::file() const noexcept { + return m_source; +} FileType IworkFile::file_type() const noexcept { return m_file_meta.type; } diff --git a/src/odr/internal/iwork/iwork_file.hpp b/src/odr/internal/iwork/iwork_file.hpp index 3e7515770..cf3f6d403 100644 --- a/src/odr/internal/iwork/iwork_file.hpp +++ b/src/odr/internal/iwork/iwork_file.hpp @@ -17,7 +17,9 @@ namespace odr::internal::iwork { /// An iWork package, detected from its archives and component list. class IworkFile final : public abstract::DocumentFile { public: - explicit IworkFile(std::shared_ptr filesystem); + /// @p source is the file the package was read from, if any. + explicit IworkFile(std::shared_ptr filesystem, + std::shared_ptr source = nullptr); [[nodiscard]] std::shared_ptr file() const noexcept override; @@ -33,6 +35,7 @@ class IworkFile final : public abstract::DocumentFile { private: std::shared_ptr m_filesystem; + std::shared_ptr m_source; FileMeta m_file_meta; }; diff --git a/src/odr/internal/odf/odf_file.cpp b/src/odr/internal/odf/odf_file.cpp index ecf97c31d..3bd3c8b5d 100644 --- a/src/odr/internal/odf/odf_file.cpp +++ b/src/odr/internal/odf/odf_file.cpp @@ -16,8 +16,10 @@ namespace odr::internal::odf { OpenDocumentFile::OpenDocumentFile( std::shared_ptr filesystem, - std::shared_ptr archive) - : m_filesystem{std::move(filesystem)}, m_archive{std::move(archive)} { + std::shared_ptr archive, + std::shared_ptr source) + : m_filesystem{std::move(filesystem)}, m_archive{std::move(archive)}, + m_source{std::move(source)} { if (m_filesystem->exists(AbsPath("/META-INF/manifest.xml"))) { const pugi::xml_document manifest = xml::parse(*m_filesystem, AbsPath("/META-INF/manifest.xml")); @@ -34,7 +36,7 @@ OpenDocumentFile::OpenDocumentFile( } std::shared_ptr OpenDocumentFile::file() const noexcept { - return {}; + return m_source; } FileType OpenDocumentFile::file_type() const noexcept { diff --git a/src/odr/internal/odf/odf_file.hpp b/src/odr/internal/odf/odf_file.hpp index acefa4d19..67087341a 100644 --- a/src/odr/internal/odf/odf_file.hpp +++ b/src/odr/internal/odf/odf_file.hpp @@ -25,9 +25,11 @@ namespace odr::internal::odf { class OpenDocumentFile final : public virtual abstract::DocumentFile { public: /// @p archive is the package @p filesystem reads from lazily, if any. + /// @p source is the file the package was read from, if any. explicit OpenDocumentFile( std::shared_ptr filesystem, - std::shared_ptr archive = nullptr); + std::shared_ptr archive = nullptr, + std::shared_ptr source = nullptr); [[nodiscard]] std::shared_ptr file() const noexcept override; @@ -51,6 +53,7 @@ class OpenDocumentFile final : public virtual abstract::DocumentFile { private: std::shared_ptr m_filesystem; std::shared_ptr m_archive; + std::shared_ptr m_source; EncryptionState m_encryption_state{EncryptionState::not_encrypted}; FileMeta m_file_meta; Manifest m_manifest; diff --git a/src/odr/internal/oldms/oldms_file.cpp b/src/odr/internal/oldms/oldms_file.cpp index f258bb325..c1d1f5f80 100644 --- a/src/odr/internal/oldms/oldms_file.cpp +++ b/src/odr/internal/oldms/oldms_file.cpp @@ -65,8 +65,9 @@ FileMeta parse_meta(const abstract::ReadableFilesystem &files) { } // namespace LegacyMicrosoftFile::LegacyMicrosoftFile( - std::shared_ptr files) - : m_files{std::move(files)} { + std::shared_ptr files, + std::shared_ptr source) + : m_files{std::move(files)}, m_source{std::move(source)} { m_file_meta = parse_meta(*m_files); // An unreadable encryption marker leaves the state unknown. @@ -79,7 +80,7 @@ LegacyMicrosoftFile::LegacyMicrosoftFile( } std::shared_ptr LegacyMicrosoftFile::file() const noexcept { - return {}; + return m_source; } FileType LegacyMicrosoftFile::file_type() const noexcept { diff --git a/src/odr/internal/oldms/oldms_file.hpp b/src/odr/internal/oldms/oldms_file.hpp index 75d8f4c82..25cfa7060 100644 --- a/src/odr/internal/oldms/oldms_file.hpp +++ b/src/odr/internal/oldms/oldms_file.hpp @@ -17,8 +17,10 @@ namespace odr::internal::oldms { class LegacyMicrosoftFile final : public abstract::DocumentFile { public: + /// @p source is the file the compound file was read from, if any. explicit LegacyMicrosoftFile( - std::shared_ptr files); + std::shared_ptr files, + std::shared_ptr source = nullptr); [[nodiscard]] std::shared_ptr file() const noexcept override; @@ -39,6 +41,7 @@ class LegacyMicrosoftFile final : public abstract::DocumentFile { private: std::shared_ptr m_files; + std::shared_ptr m_source; FileMeta m_file_meta; EncryptionState m_encryption_state{EncryptionState::unknown}; }; diff --git a/src/odr/internal/ooxml/ooxml_file.cpp b/src/odr/internal/ooxml/ooxml_file.cpp index 118d2b1d7..cc323509d 100644 --- a/src/odr/internal/ooxml/ooxml_file.cpp +++ b/src/odr/internal/ooxml/ooxml_file.cpp @@ -18,8 +18,10 @@ namespace odr::internal::ooxml { OfficeOpenXmlFile::OfficeOpenXmlFile( std::shared_ptr files, - std::shared_ptr archive) - : m_files(std::move(files)), m_archive(std::move(archive)) { + std::shared_ptr archive, + std::shared_ptr source) + : m_files(std::move(files)), m_archive(std::move(archive)), + m_source(std::move(source)) { m_file_meta = parse_file_meta(*m_files); if (m_file_meta.password_encrypted) { @@ -28,7 +30,7 @@ OfficeOpenXmlFile::OfficeOpenXmlFile( } std::shared_ptr OfficeOpenXmlFile::file() const noexcept { - return {}; + return m_source; } FileType OfficeOpenXmlFile::file_type() const noexcept { diff --git a/src/odr/internal/ooxml/ooxml_file.hpp b/src/odr/internal/ooxml/ooxml_file.hpp index b674061b0..efc5e3428 100644 --- a/src/odr/internal/ooxml/ooxml_file.hpp +++ b/src/odr/internal/ooxml/ooxml_file.hpp @@ -24,9 +24,11 @@ namespace odr::internal::ooxml { class OfficeOpenXmlFile final : public abstract::DocumentFile { public: /// @p archive is the package @p files reads from lazily, if any. + /// @p source is the file the package was read from, if any. explicit OfficeOpenXmlFile( std::shared_ptr files, - std::shared_ptr archive = nullptr); + std::shared_ptr archive = nullptr, + std::shared_ptr source = nullptr); [[nodiscard]] std::shared_ptr file() const noexcept override; @@ -50,6 +52,7 @@ class OfficeOpenXmlFile final : public abstract::DocumentFile { private: std::shared_ptr m_files; std::shared_ptr m_archive; + std::shared_ptr m_source; FileMeta m_file_meta; EncryptionState m_encryption_state{EncryptionState::not_encrypted}; }; diff --git a/src/odr/internal/open_strategy.cpp b/src/odr/internal/open_strategy.cpp index 84468034f..25245e297 100644 --- a/src/odr/internal/open_strategy.cpp +++ b/src/odr/internal/open_strategy.cpp @@ -86,8 +86,8 @@ open_file_as(const std::shared_ptr &file, const FileType as, try { auto zip_file = std::make_unique(file); auto filesystem = zip_file->archive()->as_filesystem(); - auto odf_file = - std::make_unique(filesystem, zip_file->zip()); + auto odf_file = std::make_unique( + filesystem, zip_file->zip(), file); if (odf_file->file_type() == as) { return odf_file; } @@ -116,7 +116,7 @@ open_file_as(const std::shared_ptr &file, const FileType as, try { auto zip_file = std::make_unique(file); auto filesystem = zip_file->archive()->as_filesystem(); - auto iwork_file = std::make_unique(filesystem); + auto iwork_file = std::make_unique(filesystem, file); if (iwork_file->file_type() == as) { return iwork_file; } @@ -136,7 +136,7 @@ open_file_as(const std::shared_ptr &file, const FileType as, auto zip_file = std::make_unique(file); auto filesystem = zip_file->archive()->as_filesystem(); auto ooxml_file = std::make_unique( - filesystem, zip_file->zip()); + filesystem, zip_file->zip(), file); if (is_the_requested_ooxml(*ooxml_file, as)) { return ooxml_file; } @@ -147,7 +147,8 @@ open_file_as(const std::shared_ptr &file, const FileType as, try { auto cfb_file = std::make_unique(file); auto filesystem = cfb_file->archive()->as_filesystem(); - auto ooxml_file = std::make_unique(filesystem); + auto ooxml_file = + std::make_unique(filesystem, nullptr, file); if (is_the_requested_ooxml(*ooxml_file, as)) { return ooxml_file; } @@ -166,7 +167,7 @@ open_file_as(const std::shared_ptr &file, const FileType as, auto cfb_file = std::make_unique(file); auto filesystem = cfb_file->archive()->as_filesystem(); auto oldms_file = - std::make_unique(filesystem); + std::make_unique(filesystem, file); if (oldms_file->file_type() == as) { return oldms_file; } @@ -484,7 +485,7 @@ open_by_cascade(const std::shared_ptr &file, try { ODR_VERBOSE(logger, "try open as odf"); return std::make_unique(filesystem, - zip_file->zip()); + zip_file->zip(), file); } catch (...) { ODR_VERBOSE(logger, "failed to open as odf"); } @@ -492,14 +493,14 @@ open_by_cascade(const std::shared_ptr &file, try { ODR_VERBOSE(logger, "try open as ooxml"); return std::make_unique(filesystem, - zip_file->zip()); + zip_file->zip(), file); } catch (...) { ODR_VERBOSE(logger, "failed to open as ooxml"); } try { ODR_VERBOSE(logger, "try open as iwork"); - return std::make_unique(filesystem); + return std::make_unique(filesystem, file); } catch (...) { ODR_VERBOSE(logger, "failed to open as iwork"); } @@ -515,14 +516,15 @@ open_by_cascade(const std::shared_ptr &file, try { ODR_VERBOSE(logger, "try open as legacy ms"); - return std::make_unique(filesystem); + return std::make_unique(filesystem, file); } catch (...) { ODR_VERBOSE(logger, "failed to open as legacy ms"); } try { ODR_VERBOSE(logger, "try open as ooxml"); - return std::make_unique(filesystem); + return std::make_unique(filesystem, nullptr, + file); } catch (...) { ODR_VERBOSE(logger, "failed to open as ooxml"); } diff --git a/test/src/file_test.cpp b/test/src/file_test.cpp index 35e62c4ea..bddd48953 100644 --- a/test/src/file_test.cpp +++ b/test/src/file_test.cpp @@ -136,6 +136,51 @@ TEST(File, a_file_read_into_memory_keeps_its_name) { "about.odt"); } +TEST(File, a_decoded_package_keeps_the_file_it_was_read_from) { + for (const FileType type : all_file_types()) { + if (!capabilities_by_file_type(type).create) { + continue; + } + std::ostringstream bytes; + create_document(type).save(bytes); + const std::string name = "blank." + file_type_to_string(type); + const std::size_t size = bytes.str().size(); + + for (const DecodedFile &decoded : + {open(File::from_memory(bytes.str(), name)), + open(File::from_memory(bytes.str(), name), + DecodeOptions::as(type))}) { + EXPECT_EQ(decoded.file().name(), name) << name; + EXPECT_EQ(decoded.file().size(), size) << name; + } + } +} + +/// The engines that `create_document` cannot reach: ooxml in a compound file, +/// legacy Office and iWork. +TEST(File, a_decoded_file_on_disk_keeps_its_name) { + struct Case { + const char *path; + const char *name; + }; + const std::array cases{ + Case{"odr-public/docx/encrypted.docx", "encrypted.docx"}, + Case{"odr-public/doc/file-sample_100kB.doc", "file-sample_100kB.doc"}, + Case{"odr-public/pages/empty.pages", "empty.pages"}, + }; + + for (const auto &[path, name] : cases) { + const DecodedFile decoded = open(TestData::test_file_path(path)); + EXPECT_EQ(decoded.file().name(), name) << path; + EXPECT_EQ(open(TestData::test_file_path(path), + DecodeOptions::as(decoded.file_type())) + .file() + .name(), + name) + << path; + } +} + TEST(File, copying_into_memory_uses_one_size_snapshot) { class ChangingFile final : public internal::abstract::File { public: diff --git a/wasm/tests/smoke.test.mjs b/wasm/tests/smoke.test.mjs index cf08545b8..a42e7d501 100644 --- a/wasm/tests/smoke.test.mjs +++ b/wasm/tests/smoke.test.mjs @@ -75,6 +75,22 @@ describe('smoke', () => { } }); + // A package reads through its archive, and still names the file it came from. + it('names a package upload, and leaves a new document unnamed', () => { + for (const [doc, name] of [ + [odr.open(minimalOdt(), { name: 'letter.odt' }), 'letter.odt'], + [odr.open(minimalOdt()), ''], + [odr.create(odr.enums.FileType.odt), ''], + [odr.create(odr.enums.FileType.xlsx), ''], + ]) { + try { + assert.equal(doc.fileName, name); + } finally { + doc.close(); + } + } + }); + it('throws a typed error for a wrong password', () => { const doc = odr.open(fixture('encrypted.docx')); try {