Skip to content

fix(xlsx): a part's XML prolog survives a save instead of being dropped - #425

Open
argszero wants to merge 1 commit into
iOfficeAI:mainfrom
argszero:fix/xlsx-xml-prolog-preserved
Open

argszero wants to merge 1 commit into
iOfficeAI:mainfrom
argszero:fix/xlsx-xml-prolog-preserved

Conversation

@argszero

Copy link
Copy Markdown
Contributor

Summary

The Open XML SDK models a part as its root element, so the comments and
processing instructions that sit between the XML declaration and that element
are not part of the tree. They are gone the first time the part is
re-serialized — and because the SDK re-emits the declaration itself, the loss
is silent: the saved part stays well-formed, the command exits 0, and
raw returns the root element only.

This is the xlsx half of the same defect reported in #409. It captures each
part's prolog from the package as it is opened, and re-attaches it to the
written zip. A package whose parts have the usual declaration-then-root shape
captures nothing, so those saves are byte-for-byte what they were.

Measured before and after on the same two commands, on xl/workbook.xml and
xl/worksheets/sheet1.xml:

BASE     close            xl/workbook.xml            False  got='<?xml version="1.0" encoding="utf-8"?>'
BASE     close            xl/worksheets/sheet1.xml   False  got='<?xml version="1.0" encoding="utf-8"?>'
BASE     save             xl/workbook.xml            False  got='<?xml version="1.0" encoding="utf-8"?>'
BASE     save             xl/worksheets/sheet1.xml   False  got='<?xml version="1.0" encoding="utf-8"?>'
PATCHED  close            xl/workbook.xml            True
PATCHED  close            xl/worksheets/sheet1.xml   True
PATCHED  save             xl/workbook.xml            True
PATCHED  save             xl/worksheets/sheet1.xml   True

The prolog lost is the same one in every case:
<!-- KEEP-ME: prolog note --><?xml-stylesheet type="text/xsl" href="x.xsl"?>,
immediately after the declaration and before the root element.

Where the fix lives

ExcelHandler differs from WordHandler here in a way worth naming. Both
saves and close write the file through one method — WriteBackFilteredPackage
— so the restore is attached to the single atomic write it performs, via
AtomicPackageWriter's existing postProcessTemp hook (which runs against the
temp file before the File.Replace, so the session's own handle on the original
is never in the way). The docx path needed two hooks because its two writes go
through different methods; this one needs one, and hanging the restore on the
shared write is what makes the mid-session save and the close path covered
together rather than by two independently-drifting call sites.

Core/XmlPrologPreserver.cs is the same file as in #421, byte for byte (blob
b642fb8c74466f0073f31dae8480ba07b349f63c), so if #421 lands first the two
merge cleanly and this PR reduces to the ExcelHandler change.

Tests

  • dotnet build src/officecli/officecli.csproj -c Release — 0 errors, no
    new warnings (the two pre-existing ones are unchanged)
  • A self-written A/B script, base vs patched, failing before the change
    (6/6 cases lost the prolog) and passing after (6/6 preserved), on both the
    save and the close write path
  • Inertness: on a package with no prolog anywhere, the patched build's
    entries are identical to the base build's once the save timestamp is
    normalised — nothing is rewritten that did not need to be
  • The repository has no test suite, so there is no suite to run; the A/B
    script above is the verification

Open question

I read Rule 1 as "one format per PR", so this is the xlsx half alone and the pptx half (ppt/presentation.xml, which still drops the prolog) would follow separately. I did ask on #409 whether you would rather have the three formats in one PR or one each, and had no answer, so I picked the reading that keeps each change independently revertable. Say the word and I will either fold pptx in here or open it on its own.

Refs #409 (xlsx; pptx still drops it).

The Open XML SDK models a part as its root element, so the comments and
processing instructions between the XML declaration and that element are not
part of the tree and are gone the first time the part is re-serialized. The
declaration itself survives (the SDK re-emits it), so the save is silent: the
part stays well-formed, the command exits 0, and `raw` returns the root element
only. Measured on xl/workbook.xml and xl/worksheets/sheet1.xml.

Capture each part's prolog from the package as opened, and re-attach it to the
written zip from WriteBackFilteredPackage -- the single method both the
mid-session flush and the close-time write go through, which is why the restore
hangs on AtomicPackageWriter's postProcessTemp hook rather than on two separate
call sites. A package whose parts have the usual declaration-then-root shape
captures nothing, so those saves are byte-for-byte what they were.

Refs iOfficeAI#409 (xlsx; pptx still drops it).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant