feat(odf): decode flat xml OpenDocument files - #731
Open
andiwand wants to merge 2 commits into
Open
Conversation
`.fodt`, `.fodp`, `.fods` and `.fodg` were only extension and mimetype aliases on the zip-backed rows, so the table promised `open`, `translate_html`, `edit` and `save` for a file that in fact fell through to the xml source view - or, when a host trusted the extension and named the type, threw `NoOpenDocumentFile`. A flat document's one `office:document` root carries what `content.xml` and `styles.xml` carry between them, so `odf::Document` gains a constructor that hands that root in as both roots; parse, styles, list numbering and the adapter are the existing ones. Recognition rides on the parse `XmlFile` already did, next to svg's - the root element is all that tells either from any other xml. Without a package there is no filesystem, so two things differ. Images come base64 encoded in `office:binary-data`, decoded lazily by the adapter, which also picks them up in a package where they are equally legal. And `save` re-serialises the one tree rather than rebuilding a zip. Verified against LibreOffice's own flat export of eleven files across the four types: the rendered html matches the packaged render line for line, with byte-identical image payloads, and what differs is what LibreOffice itself writes differently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LUmsvGsbfXFgubV2RcpR4P
`office:binary-data` and `xlink:href` can sit on the same `draw:image`. The short-circuit that reads the markup bytes returned `is_internal()` before the `make_absolute()` check that rejects an escaping path, while `image_href` still answered with the raw attribute - so `bring_offline` wrote the decoded bytes to wherever the href pointed. The bytes in the markup are the image, so an href beside them names no file of ours and never reaches the renderer now. Also from the same review: - `Document::as_filesystem` answers with an empty filesystem where the impl has none instead of throwing `std::invalid_argument`, which every flat document did - `odr-cli server foo.fodt` died on it after having built its html service. The internal null stays null: `save` and the image lookups pick the flat path off it. - `parse_file_meta` shares `read_entry_count` and `document_type_by_file_type` with the flat parse rather than writing both out by hand. - Eight tests: `office:binary-data` in a package, markup bytes beating an href in both encodings, the entry counts including a negative, `list_file_types`, the empty filesystem, distinct hrefs, and a real `html::translate` of a flat document embedded and linked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015E5WrxdYxfoBE7JUotBtqh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.fodt,.fodp,.fodsand.fodgwere only extension and mimetype aliases on the zip-backed rows. The table therefore promisedopen,translate_html,editandsavefor a file that in fact fell through to the xml source view — and a host that trusted the extension and named the type gotNoOpenDocumentFileinstead. This makes the promise true.How
A flat document's one
office:documentroot carries whatcontent.xmlandstyles.xmlcarry between them, soodf::Documentgains a constructor that hands that root in as both roots. Everything downstream —parse_tree,StyleRegistry, list numbering, the adapter — is the existing code, shared with the packaged path via a newinit_.Recognition rides on the parse
xml::XmlFilealready did, right next to svg's: the root element is the only thing that tells either from any other xml.is_flat_opendocument_filereads it off that tree;FlatOpenDocumentFilethen reparses with the document parse options, which a source view's (parse_full | parse_ws_pcdata_single) cannot substitute for — whitespace-only pcdata is text in one and nothing in the other.Without a package there is no filesystem, so two things differ:
office:binary-data. The adapter decodes them lazily, and does so in a package too, where they are equally legal but were previously reported as external.savere-serialises the one tree instead of rebuilding a zip.Flat files are never encrypted, so
decryptthrowsNotEncryptedError.Verification
Ten unit tests on inline flat-xml literals cover the four types, the recogniser (including what must not open as flat), tree and style resolution from the single root, embedded and linked images, and the save round-trip.
Beyond that, LibreOffice's own flat export of eleven files across the four types was rendered and compared against the packaged render of the same source:
Image payloads are byte-identical throughout. The remaining line differences trace to LibreOffice's export, not to the decoder — e.g. it writes
svg:stroke-color="#000000" draw:fill-color="#99ccff"into the default graphic style that the packaged file does not have.Full suite: 995 passed, 0 failed, 8 skipped (the usual svm/wpd ones).
Follow-up, not in here
The corpus in
OpenDocument.testhas no flat files, sohtml_output_testgives this no pinned reference coverage. Adding one file per type there plus a pin bump is the natural next step — happy to do it as a stacked change if you want it.