Skip to content

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

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

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

Conversation

@argszero

@argszero argszero commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

WordHandler re-serializes each part through the Open XML SDK on every save. The SDK models a part as its root element, and a prolog — the comments and processing instructions between the XML declaration and that element — is not part of that tree, so the first write drops it. The declaration itself survives, because the SDK re-emits it, and that is what makes this silent: the saved part is still well-formed, the command still exits 0, and raw returns the root element only, so nothing on the read surface mentions what was lost.

Measured on a build of main before this change, one ordinary edit each, rc=0 in every row:

part prolog before after rc
word/document.xml 1 0 0
word/styles.xml 1 0 0

The declaration is byte-identical across the edit, so the diff is exactly the prolog — both a comment and a <?xml-stylesheet?> PI in one write:

-<?xml version="1.0" encoding="utf-8"?><!-- KEEP-ME: prolog note --><?xml-stylesheet type="text/xsl" href="x.xsl"?><w:document …>
+<?xml version="1.0" encoding="utf-8"?><w:document …>

What changed

Two files. A new Core/XmlPrologPreserver.cs, and three lines in WordHandler:

  • Capture reads the prolog out of every XML part of the package as opened; Restore re-attaches it to the written zip. Both share one scanner for the declaration / misc / root shape, and Restore skips any entry that still has a prolog of its own, so a part the SDK copied through verbatim is never given a second copy.
  • Capture runs at open, against the in-memory package copy — the only point where every part still carries the prolog the author wrote.
  • Restore runs in both places that write the file: the mid-session flush (Save) and the close-time AtomicWriteBack, next to the existing FlushPendingWholeParts / NormalizeSelfClosingInDocx rewrites.

Why the mid-session path needed its own hook — measured, not assumed: on the unpatched build a resident set followed by save (no close) loses the prolog, and save is a persist path a caller can stop at. The neighbouring rewrites are deliberately skipped there on the understanding that they need the file unlocked; the post-process actually runs against the temp file before the swap (AtomicPackageWriter.Flush writes the temp, calls postProcessTemp, releases the lock, then File.Replace), so it is safe at save time too. I wired the restore that way and left the neighbouring steps alone — that is a separate concern and this PR should stay one.

Rule 1 self-check

Asked of this diff — can it be decomposed into PRs that could be merged or reverted independently? — no. The helper has no consumer until the hooks land, so alone it would be dead code; the hooks alone would not compile against it. One root cause, one fix. The format axis is genuinely separable and is deliberately left out — see Scope.

Validation

verify409.py — unpatched build: exit 1, 5 checks fail. Patched build: exit 0, all pass. It asserts the prolog is byte-for-byte part of the document header after an edit, survives a mid-session save too, that a part with no prolog is untouched, and that the edit still lands and validate stays clean:

#!/usr/bin/env python3
"""Verification for the docx XML-prolog change (issue #409).

Usage: verify409.py <officecli-binary>
"""
import os, shutil, subprocess, sys, zipfile

BIN = sys.argv[1]
WORK = os.path.join(os.path.dirname(os.path.abspath(__file__)), "verify-work")
PART = "word/document.xml"
MARK = "<!-- KEEP-ME: prolog note -->"
PI = '<?xml-stylesheet type="text/xsl" href="x.xsl"?>'
PROLOG = MARK + PI

if os.path.isdir(WORK):
    shutil.rmtree(WORK)
os.makedirs(WORK)
os.chdir(WORK)

fails = []


def run(*a):
    return subprocess.run([BIN, *a], capture_output=True, text=True)


def read_part(path, part):
    with zipfile.ZipFile(path) as z:
        return z.read(part).decode("utf-8")


def write_part(path, part, text):
    tmp = path + ".t"
    with zipfile.ZipFile(path) as zin, zipfile.ZipFile(tmp, "w", zipfile.ZIP_DEFLATED) as zo:
        for it in zin.infolist():
            d = zin.read(it.filename)
            if it.filename == part:
                d = text.encode("utf-8")
            zo.writestr(it, d)
    os.replace(tmp, path)


def header(xml):
    """Everything up to the root element's opening '<' — declaration + prolog."""
    i = 1 if xml.startswith("\ufeff") else 0
    if xml.startswith("<?xml", i):
        i = xml.index("?>", i) + 2
    while i < len(xml):
        if xml[i].isspace():
            i += 1
            continue
        if xml.startswith("<!--", i):
            i = xml.index("-->", i) + 3
            continue
        if xml.startswith("<?", i):
            i = xml.index("?>", i) + 2
            continue
        break
    return xml[:i]


def check(name, cond, detail=""):
    print(f"  {'PASS' if cond else 'FAIL'}  {name}" + (f" — {detail}" if detail else ""))
    if not cond:
        fails.append(name)


def make_docx(fname):
    if os.path.exists(fname):
        os.remove(fname)
    run("create", fname)
    run("add", fname, "/body", "--type", "paragraph", "--prop", "text=hello")
    run("close", fname)


print(f"binary: {BIN}\n")

print("[1] prolog survives an ordinary edit, byte-for-byte")
make_docx("a.docx")
orig = read_part("a.docx", PART)
injected = orig.replace("?>", "?>" + PROLOG, 1)
write_part("a.docx", PART, injected)
want_header = header(injected)

r = run("set", "a.docx", "/body/p[1]", "--prop", "bold=true")
run("close", "a.docx")
got = read_part("a.docx", PART)
check("command exits 0", r.returncode == 0, f"rc={r.returncode}")
check("prolog content still present", MARK in got and PI in got)
check("header byte-identical (declaration + prolog)", header(got) == want_header,
      f"want {want_header!r} got {header(got)!r}")
check("root element follows the prolog", got[len(want_header):].startswith("<w:document"))

print("[2] prolog survives a mid-session `save` (resident still open)")
make_docx("b.docx")
write_part("b.docx", PART, read_part("b.docx", PART).replace("?>", "?>" + PROLOG, 1))
run("open", "b.docx")
run("set", "b.docx", "/body/p[1]", "--prop", "italic=true")
s = run("save", "b.docx")
check("`save` exits 0", s.returncode == 0, f"rc={s.returncode}")
check("prolog present after `save` (before close)", MARK in read_part("b.docx", PART))
run("close", "b.docx")
check("prolog present after `close`", MARK in read_part("b.docx", PART))

print("[3] inert where there is no prolog content")
make_docx("c.docx")
c_orig = read_part("c.docx", PART)
h = header(c_orig)
check("no-prolog fixture has an empty prolog to begin with", h == c_orig[:c_orig.index("<w:document")])
run("set", "c.docx", "/body/p[1]", "--prop", "bold=true")
run("close", "c.docx")
c_after = read_part("c.docx", PART)
check("header unchanged", header(c_after) == h, f"{h!r} -> {header(c_after)!r}")
check("no prolog introduced", "<!--" not in header(c_after) and "<?xml-stylesheet" not in c_after)

make_docx("d.docx")
d_orig = read_part("d.docx", PART)
write_part("d.docx", PART, d_orig.replace("?>", "?>\n", 1))
run("set", "d.docx", "/body/p[1]", "--prop", "bold=true")
run("close", "d.docx")
check("whitespace-only prolog is not 'content' (no rewrite, file stays valid)",
      run("validate", "d.docx").returncode == 0)

print("[4] the edit still lands and the package still validates")
g = run("get", "a.docx", "/body/p[1]", "--json")
check("edited property is readable back", g.returncode == 0 and "bold" in g.stdout, g.stdout.strip()[:80])
v = run("validate", "a.docx")
check("validate clean on the prolog-bearing file", v.returncode == 0, v.stdout.strip()[:120])
w = run("validate", "b.docx")
check("validate clean on the saved file", w.returncode == 0, w.stdout.strip()[:120])

print()
if fails:
    print(f"FAILED ({len(fails)}): " + "; ".join(fails))
    sys.exit(1)
print("ALL CHECKS PASSED")

Is the fix inert where there is no prolog?

Six fixtures, built and edited with both binaries, comparing the bytes of every *.xml / *.rels part. The differing set for the two no-prolog fixtures is exactly the timestamp carriers (docProps/core.xml, the audit stamp, .rels) — which is also the noise floor measured by running the same binary twice:

fixture entries differing (patched vs unpatched) word/document.xml sha256
docx-no-prolog _rels/.rels, docProps/core.xml, docProps/custom.xml, word/_rels/document.xml.rels — = the timestamp noise floor 78a3527d8ef86851 identical on all three runs
docx-ws-only-prolog same set — no rewrite triggered 78a3527d8ef86851 identical
docx-prolog the above + word/document.xml 78a3527d8ef86851 → df06967112416e45 (the prolog)
docx-styles-prolog the above + word/styles.xml; document.xml untouched a9cd7030e0ed85e8, unchanged

And no duplication, on any part

A part the SDK never models is copied through with its prolog intact; if Restore failed to notice, it would insert a second copy. Injecting the same prolog into every XML part of a docx and then editing it leaves all 11 parts at exactly one copy, validate clean:

ok  [Content_Types].xml              before=1 after=1      ok  word/document.xml       before=1 after=1
ok  _rels/.rels                      before=1 after=1      ok  word/numbering.xml      before=1 after=1
ok  docProps/app.xml                 before=1 after=1      ok  word/settings.xml       before=1 after=1
ok  docProps/core.xml                before=1 after=1      ok  word/styles.xml         before=1 after=1
ok  docProps/custom.xml              before=1 after=1      ok  word/theme/theme1.xml   before=1 after=1
ok  word/_rels/document.xml.rels     before=1 after=1

Surrounding surfaces, patched vs unpatched

Same commands on both binaries; every field identical except the one this PR targets:

check patched unpatched
raw <f> /word/document.xml still element-only, prolog hidden ✅ ✅ (unchanged by design)
dump → batch round trip (13 items) rc 0, validate 0 rc 0, validate 0
batch on a prolog-bearing file keeps prolog drops it
xlsx / pptx smoke ok ok
read-only session (get + validate) leaves the file's bytes untouched ✅ ✅

Scope

Docx only — #409 stays open after this. It fixes docx, and it is one format because Rule 1 asks for independently mergeable pieces: xlsx and pptx still drop the prolog on this branch (measured with the same harness — xl/worksheets/sheet1.xml 1→0, ppt/slides/slide1.xml 1→0, both rc=0). The helper is format-agnostic, so each follow-up is a capture call in that handler's open path and a restore call in its write path; I would rather land one format at a time than one PR that has to be reverted across three. Say the word if you would prefer them all in one PR instead.

Not in scope

  • A DOCTYPE in the prolog is not traversed — DTDs are prohibited in OOXML (the same reason WorksheetBloatFilter drops them) and its internal subset needs real parsing to skip safely. The scan stops there, so comments/PIs after a DOCTYPE are not restored either; the part is then left exactly as the SDK wrote it.
  • Whitespace-only between the declaration and the root is not treated as content — a consumer loses nothing, and treating it as prolog would force a zip rewrite of every pretty-printed part.

Refs #409.

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.

Capture each part's prolog from the package as opened, and re-attach it to the
written zip in both places that write the file: the mid-session flush and the
close-time atomic write (next to the existing whole-part and self-closing
rewrites). 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 (docx; xlsx/pptx still drop 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