From 3188bdc248f85dc9fee5625c1c8594ded79be077 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 2 Sep 2026 20:25:58 +0200 Subject: [PATCH] feat(file): hand out the thumbnail the package already carries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DocumentFile::thumbnail()` returns the preview the producing application stored: ODF's `Thumbnails/thumbnail.png`, and for OOXML the part the package relationship names — `.jpeg`, `.png`, `.emf` and `.wmf` all occur, so the relationship is read rather than a filename guessed. Nothing is rendered; a package that carries none says so, and an encrypted one carries none until it is decrypted. `parse_relationship_target` learned the package root, which is not a part and so has no parent to hang `_rels` off; `/_rels/.rels` is where its relationships live and its targets resolve against the root. Mirrored in the JNI, Apple and Python bindings, each with a test — the shared `mixed-layout.odt` fixture is real LibreOffice output and carries one. Closes #21. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018cMYRXLJdiCkH65Jm2W9B5 --- CHANGELOG.md | 3 ++ apple/include/OdrCoreObjC/ODRFile.h | 3 ++ apple/src/ODRFile.mm | 11 ++++ apple/tests/OdrCoreTests.swift | 8 +++ .../app/opendocument/core/DocumentFile.java | 11 ++++ jni/src/jni_file.cpp | 12 +++++ jni/tests/app/opendocument/core/FileTest.java | 11 ++++ python/src/bind_file.cpp | 3 ++ python/tests/test_file.py | 19 +++++++ src/odr/file.cpp | 8 +++ src/odr/file.hpp | 7 +++ src/odr/internal/abstract/file.hpp | 3 ++ src/odr/internal/odf/odf_file.cpp | 10 ++++ src/odr/internal/odf/odf_file.hpp | 2 + src/odr/internal/ooxml/ooxml_file.cpp | 15 ++++++ src/odr/internal/ooxml/ooxml_file.hpp | 2 + src/odr/internal/ooxml/ooxml_util.cpp | 9 +++- test/src/file_test.cpp | 53 +++++++++++++++++++ 18 files changed, 189 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e12e3643b..a998fa6fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- New `DocumentFile::thumbnail()`, the preview the package carries or + `nullopt`, mirrored in the JNI, Apple and Python bindings. Closes #21. + - **Breaking** (Swift only): `HtmlConfig`'s optional settings are Swift optionals of the real type rather than `NSNumber`/`NSValue` boxes — `spreadsheetLimit` is a `TableDimensions?`, `initialZoom` a `Double?`, and so diff --git a/apple/include/OdrCoreObjC/ODRFile.h b/apple/include/OdrCoreObjC/ODRFile.h index 0895e5eba..5516194c6 100644 --- a/apple/include/OdrCoreObjC/ODRFile.h +++ b/apple/include/OdrCoreObjC/ODRFile.h @@ -323,6 +323,9 @@ NS_SWIFT_NAME(DocumentFile) - (nullable instancetype)initWithPath:(NSString *)path error:(NSError **)error; @property(nonatomic, readonly) ODRDocumentType documentType; +/// The preview image the package carries, `nil` where it carries none or is +/// still encrypted. Never rendered by us. +@property(nonatomic, readonly, nullable) ODRFile *thumbnail; - (nullable ODRDocumentFile *)decryptWithPassword:(NSString *)password error:(NSError **)error; /// Decodes the document. The expensive step. diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index af31d340a..2b52aaca4 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -588,6 +588,17 @@ - (ODRDocumentType)documentType { ODRDocumentTypeUnknown); } +- (nullable ODRFile *)thumbnail { + return guarded_value( + [&]() -> ODRFile * { + const std::optional thumbnail = + self.documentHandle.thumbnail(); + return thumbnail.has_value() ? [ODRFile fileWithHandle:*thumbnail] + : nil; + }, + nil); +} + - (nullable ODRDocumentFile *)decryptWithPassword:(NSString *)password error:(NSError **)error { return guarded(error, [&]() -> ODRDocumentFile * { diff --git a/apple/tests/OdrCoreTests.swift b/apple/tests/OdrCoreTests.swift index 0bf5076b5..0d8d947e9 100644 --- a/apple/tests/OdrCoreTests.swift +++ b/apple/tests/OdrCoreTests.swift @@ -94,6 +94,14 @@ final class DecodeTests: XCTestCase { } } +final class ThumbnailTests: XCTestCase { + func testDocumentFileCarriesItsThumbnail() throws { + let file = try DecodedFile.decode(path: try Fixture.odt()).asDocumentFile() + let thumbnail = try XCTUnwrap(file.thumbnail) + XCTAssertGreaterThan(try thumbnail.data().count, 0) + } +} + final class HtmlTests: XCTestCase { private func service() throws -> HtmlService { let file = try DecodedFile.decode(path: try Fixture.odt()) diff --git a/jni/java/app/opendocument/core/DocumentFile.java b/jni/java/app/opendocument/core/DocumentFile.java index 08cb985d8..f850fbfb5 100644 --- a/jni/java/app/opendocument/core/DocumentFile.java +++ b/jni/java/app/opendocument/core/DocumentFile.java @@ -28,6 +28,15 @@ public DocumentFile decrypt(String password) { return new DocumentFile(decryptDocumentFileNative(handle(), password)); } + /** + * The preview image the package carries, or {@code null} where it carries + * none or is still encrypted. Never rendered by us. + */ + public File thumbnail() { + long handle = thumbnailNative(handle()); + return handle == 0 ? null : new File(handle); + } + public Document document() { return new Document(documentNative(handle())); } @@ -42,5 +51,7 @@ public Document document() { private native long decryptDocumentFileNative(long handle, String password); + private native long thumbnailNative(long handle); + private native long documentNative(long handle); } diff --git a/jni/src/jni_file.cpp b/jni/src/jni_file.cpp index b55218ccf..1f5ca6b3c 100644 --- a/jni/src/jni_file.cpp +++ b/jni/src/jni_file.cpp @@ -7,6 +7,7 @@ #include #include +#include #include namespace { @@ -365,6 +366,17 @@ Java_app_opendocument_core_DocumentFile_documentTypeNative(JNIEnv *env, jobject, }); } +/// 0 where there is no thumbnail; `DocumentFile.thumbnail` maps that to null. +extern "C" JNIEXPORT jlong JNICALL +Java_app_opendocument_core_DocumentFile_thumbnailNative(JNIEnv *env, jobject, + jlong handle) { + return guarded(env, [&]() -> jlong { + const std::optional thumbnail = + decoded(handle).as_document_file().thumbnail(); + return thumbnail.has_value() ? make_handle(*thumbnail) : 0; + }); +} + extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_DocumentFile_decryptDocumentFileNative( JNIEnv *env, jobject, jlong handle, jstring password) { diff --git a/jni/tests/app/opendocument/core/FileTest.java b/jni/tests/app/opendocument/core/FileTest.java index b23be8340..878ce9723 100644 --- a/jni/tests/app/opendocument/core/FileTest.java +++ b/jni/tests/app/opendocument/core/FileTest.java @@ -29,6 +29,17 @@ void openOdt() throws IOException { } } + @Test + void thumbnail() throws IOException { + Path odt = TestFiles.odtFile(tempDir); + try (DecodedFile file = Odr.open(odt.toString())) { + try (File thumbnail = file.asDocumentFile().thumbnail()) { + assertNotNull(thumbnail); + assertTrue(thumbnail.read().length > 0); + } + } + } + @Test void openText() throws IOException { Path txt = TestFiles.txtFile(tempDir); diff --git a/python/src/bind_file.cpp b/python/src/bind_file.cpp index 360f8994d..57f7985bc 100644 --- a/python/src/bind_file.cpp +++ b/python/src/bind_file.cpp @@ -282,6 +282,9 @@ void odr_python::bind_file(py::module_ &m) { .def("document_type", &odr::DocumentFile::document_type) .def("decrypt", &odr::DocumentFile::decrypt, py::arg("password"), py::call_guard()) + .def("thumbnail", &odr::DocumentFile::thumbnail, + "The preview image the package carries, or `None` where it " + "carries none or is still encrypted. Never rendered by us.") .def("document", &odr::DocumentFile::document); py::class_(m, "PdfFile") diff --git a/python/tests/test_file.py b/python/tests/test_file.py index b6b3978aa..4527af632 100644 --- a/python/tests/test_file.py +++ b/python/tests/test_file.py @@ -1,3 +1,5 @@ +import zipfile + import pytest import pyodr @@ -158,6 +160,23 @@ def test_document_file_from_disk_and_from_memory(odt_path): ) +def test_document_file_thumbnail(tmp_path, odt_path): + # The minimal odt the fixture builds carries none. + assert pyodr.DocumentFile.from_disk(str(odt_path)).thumbnail() is None + + with_thumbnail = tmp_path / "with-thumbnail.odt" + with zipfile.ZipFile(odt_path) as source: + entries = {name: source.read(name) for name in source.namelist()} + entries["Thumbnails/thumbnail.png"] = b"not really a png" + with zipfile.ZipFile(with_thumbnail, "w") as archive: + for name, content in entries.items(): + archive.writestr(name, content) + + thumbnail = pyodr.DocumentFile.from_disk(str(with_thumbnail)).thumbnail() + assert thumbnail is not None + assert thumbnail.read() == b"not really a png" + + def test_document_file_from_memory_rejects_a_non_document(): with pytest.raises(pyodr.Error): pyodr.DocumentFile.from_memory(b"not a document") diff --git a/src/odr/file.cpp b/src/odr/file.cpp index e3ffbe9c6..6d2f28668 100644 --- a/src/odr/file.cpp +++ b/src/odr/file.cpp @@ -413,6 +413,14 @@ DocumentFile DocumentFile::decrypt(const std::string &password) const { return DecodedFile::decrypt(password).as_document_file(); } +std::optional DocumentFile::thumbnail() const { + if (const std::shared_ptr thumbnail = + m_impl->thumbnail()) { + return File(thumbnail); + } + return {}; +} + Document DocumentFile::document() const { return Document(m_impl->document()); } std::shared_ptr DocumentFile::impl() const { diff --git a/src/odr/file.hpp b/src/odr/file.hpp index 7b2532952..fd8ba11d2 100644 --- a/src/odr/file.hpp +++ b/src/odr/file.hpp @@ -540,6 +540,13 @@ class DocumentFile final : public DecodedFile { [[nodiscard]] DocumentFile decrypt(const std::string &password) const; + /// @brief The preview image the package carries, if any. + /// + /// What the producing application stored — never rendered by us, so it is + /// only as current as the last save. Empty for a package that carries none + /// or is still encrypted. + [[nodiscard]] std::optional thumbnail() const; + [[nodiscard]] Document document() const; [[nodiscard]] std::shared_ptr impl() const; diff --git a/src/odr/internal/abstract/file.hpp b/src/odr/internal/abstract/file.hpp index 54131beaa..2fc99e65d 100644 --- a/src/odr/internal/abstract/file.hpp +++ b/src/odr/internal/abstract/file.hpp @@ -93,6 +93,9 @@ class DocumentFile : public DecodedFile { [[nodiscard]] virtual DocumentType document_type() const = 0; + /// The preview the package carries, `nullptr` where there is none. + [[nodiscard]] virtual std::shared_ptr thumbnail() const { return {}; } + [[nodiscard]] virtual std::shared_ptr document() const = 0; }; diff --git a/src/odr/internal/odf/odf_file.cpp b/src/odr/internal/odf/odf_file.cpp index b4c7657f8..bc8685fc5 100644 --- a/src/odr/internal/odf/odf_file.cpp +++ b/src/odr/internal/odf/odf_file.cpp @@ -50,6 +50,16 @@ DocumentType OpenDocumentFile::document_type() const { return m_file_meta.document_type; } +std::shared_ptr OpenDocumentFile::thumbnail() const { + // [OpenDocument] 3.9. Encrypted alongside everything else. + static const AbsPath path("/Thumbnails/thumbnail.png"); + if (m_encryption_state == EncryptionState::encrypted || + !m_filesystem->is_file(path)) { + return {}; + } + return m_filesystem->open(path); +} + bool OpenDocumentFile::password_encrypted() const noexcept { return m_file_meta.password_encrypted; } diff --git a/src/odr/internal/odf/odf_file.hpp b/src/odr/internal/odf/odf_file.hpp index 07ad105b1..74b0ce461 100644 --- a/src/odr/internal/odf/odf_file.hpp +++ b/src/odr/internal/odf/odf_file.hpp @@ -31,6 +31,8 @@ class OpenDocumentFile final : public virtual abstract::DocumentFile { [[nodiscard]] DocumentType document_type() const override; + [[nodiscard]] std::shared_ptr thumbnail() const override; + [[nodiscard]] bool password_encrypted() const noexcept override; [[nodiscard]] EncryptionState encryption_state() const noexcept override; [[nodiscard]] std::shared_ptr diff --git a/src/odr/internal/ooxml/ooxml_file.cpp b/src/odr/internal/ooxml/ooxml_file.cpp index f60bd58ae..f7cce5713 100644 --- a/src/odr/internal/ooxml/ooxml_file.cpp +++ b/src/odr/internal/ooxml/ooxml_file.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -43,6 +44,20 @@ DocumentType OfficeOpenXmlFile::document_type() const { return m_file_meta.document_type; } +std::shared_ptr OfficeOpenXmlFile::thumbnail() const { + // [ECMA-376] 15.2.10. Named by relationship, not by convention: `.jpeg`, + // `.png`, `.emf` and `.wmf` all occur. + if (m_encryption_state == EncryptionState::encrypted) { + return {}; + } + const std::optional path = + parse_relationship_target(*m_files, AbsPath("/"), "thumbnail"); + if (!path.has_value() || !m_files->is_file(*path)) { + return {}; + } + return m_files->open(*path); +} + bool OfficeOpenXmlFile::password_encrypted() const noexcept { return m_file_meta.password_encrypted; } diff --git a/src/odr/internal/ooxml/ooxml_file.hpp b/src/odr/internal/ooxml/ooxml_file.hpp index 5bc9a0faf..a897bc53c 100644 --- a/src/odr/internal/ooxml/ooxml_file.hpp +++ b/src/odr/internal/ooxml/ooxml_file.hpp @@ -30,6 +30,8 @@ class OfficeOpenXmlFile final : public abstract::DocumentFile { [[nodiscard]] DocumentType document_type() const override; + [[nodiscard]] std::shared_ptr thumbnail() const override; + [[nodiscard]] bool password_encrypted() const noexcept override; [[nodiscard]] EncryptionState encryption_state() const noexcept override; [[nodiscard]] std::shared_ptr diff --git a/src/odr/internal/ooxml/ooxml_util.cpp b/src/odr/internal/ooxml/ooxml_util.cpp index e0d99cc49..649b27787 100644 --- a/src/odr/internal/ooxml/ooxml_util.cpp +++ b/src/odr/internal/ooxml/ooxml_util.cpp @@ -339,7 +339,13 @@ ooxml::parse_relationships(const pugi::xml_document &relations) { namespace { +/// The root is not a part, so it has no parent to hang `_rels` off. +bool is_package_root(const AbsPath &path) { return path == AbsPath("/"); } + AbsPath relationships_path(const AbsPath &path) { + if (is_package_root(path)) { + return AbsPath("/_rels/.rels"); + } return path.parent() .join(RelPath("_rels")) .join(RelPath(path.basename() + ".rels")); @@ -357,7 +363,8 @@ std::optional resolve_relationship_target(const AbsPath &path, return AbsPath(target); } try { - return path.parent().join(RelPath(target)); + const AbsPath base = is_package_root(path) ? path : path.parent(); + return base.join(RelPath(target)); } catch (const std::invalid_argument &) { return {}; } diff --git a/test/src/file_test.cpp b/test/src/file_test.cpp index 147e3a21a..c36e465d3 100644 --- a/test/src/file_test.cpp +++ b/test/src/file_test.cpp @@ -4,10 +4,14 @@ #include #include +#include +#include #include #include +#include +#include #include #include @@ -175,6 +179,55 @@ TEST(DocumentFile, from_memory_throws_on_a_non_document) { NoDocumentFile); } +TEST(DocumentFile, odf_thumbnail) { + const DocumentFile file( + TestData::test_file_path("odr-public/ods/file_example_ODS_10.ods")); + + const std::optional thumbnail = file.thumbnail(); + ASSERT_TRUE(thumbnail.has_value()); + EXPECT_LT(0, thumbnail->size()); + EXPECT_EQ(DecodedFile(*thumbnail).file_type(), + FileType::portable_network_graphics); +} + +TEST(DocumentFile, thumbnail_is_absent_where_the_package_has_none) { + const DocumentFile file( + TestData::test_file_path("odr-public/docx/style-various-1.docx")); + + EXPECT_FALSE(file.thumbnail().has_value()); +} + +/// The name is deliberately unconventional: only reading the relationship +/// finds it. +TEST(DocumentFile, ooxml_thumbnail_is_named_by_the_package_relationship) { + internal::zip::ZipArchive zip; + zip.insert_file( + std::end(zip), internal::RelPath("_rels/.rels"), + std::make_shared( + R"()" + R"()" + R"()" + R"()")); + zip.insert_file(std::end(zip), internal::RelPath("word/document.xml"), + std::make_shared( + R"()")); + zip.insert_file(std::end(zip), internal::RelPath("docProps/preview.emf"), + std::make_shared("not really an emf")); + + std::stringstream out; + zip.save(out); + + const DocumentFile file = DocumentFile::from_memory(out.str()); + ASSERT_EQ(file.file_type(), FileType::office_open_xml_document); + + const std::optional thumbnail = file.thumbnail(); + ASSERT_TRUE(thumbnail.has_value()); + EXPECT_EQ(internal::util::stream::read(*thumbnail->stream()), + "not really an emf"); +} + TEST(DecodedFile, wpd) { const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose);