Conversation
A story splice into the main part, such as insert_content or insert_picture_to_story, edits the canonical XML that story_sources builds with CT_Document::to_xml. That XML never declares a root default namespace or a namespace on w:body, and always binds the root w, r and mc prefixes to their canonical URIs. The splice therefore published the main part without the producer's root default and body declarations, and with those prefixes rebound. The save path refuses all three cases, but the splice path never ran that check. Without a content control in the body, every splice was accepted. An element such as <producer/> under xmlns="urn:used-default", or <r:producer/> under a non-canonical xmlns:r, silently changed namespace, and an <x:producer/> nested in a run under <w:body xmlns:x="urn:x"> was written with an unbound prefix. With a content control, the picture and append paths refused only by accident, through stale namespace facts, and insert_content was still accepted. set_story_source_xml now runs unsafe_serializer_namespace_prefix, the check of the save path, on the root default, the root w, r and mc declarations and the w:body declarations of the part it replaces, before it publishes a main-part splice. An unused default, like the task namespace Google Docs declares, still passes. The other fixed root prefixes stay out of the check because the canonical root replays their producer values unchanged. Any w:body declaration refuses, as it does for a modified save, because the canonical writer does not carry it onto a raw element nested in a paragraph or run. GitHub issue tensorbee#157.
Python Document.add_picture failed with "cannot serialize a modified document with a shadowed `default` namespace" on a Google Docs export, whose document.xml root declares an unused default namespace and whose body holds a content control. add_picture calls insert_picture_to_story, which splices the picture into canonical main-part XML without the root default and publishes it through set_story_source_xml. That function replaced the part and the typed document but kept the root and body namespace facts of the part it replaced. The staged package preparation then re-serializes the typed body, and a content control does not survive that round trip unchanged, so the flush that follows ran its namespace check with the stale facts against the spliced bytes. The recorded default was missing from those bytes, a case the check cannot classify, so it refused. set_story_source_xml now refreshes the three facts from the bytes it publishes, as flush_document_to_package does after a canonical publication. Body, table-cell and text-box splices into the main part all go through it. The previous commit refuses a used root default, a root w, r or mc prefix bound to another URI and any w:body declaration before a splice publishes, so the refresh cannot turn their accidental refusal into a silent namespace change. GitHub issue tensorbee#157.
The fail-closed check and the namespace facts refresh in set_story_source_xml, with their regression tests, grow the rdocx package, so the README archive row and its ARCHIVE_MEASUREMENTS entry are re-measured. GitHub issue tensorbee#157.
This branch has not been deployed
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.
Summary
rdocx: beforeset_story_source_xmlpublishes a story splice into themain part, it runs
unsafe_serializer_namespace_prefix, the check of thesave path, on the root default, the root
w,randmcdeclarations andthe
w:bodydeclarations of the part it replaces. A used root default, orone whose use cannot be classified, is refused with
cannot serialize a modified document with a shadowed `default` namespace. A rootw,rormcprefix bound to another URI, and any declaration onw:body, arerefused with the same error naming that prefix. An unused default, like the
Google Docs task namespace, passes (157-3).
rdocx: after a main-part splice,set_story_source_xmlrefreshesroot_namespace_declarations,body_namespace_declarationsandbody_namespace_bindingsfrom the bytes it publishes, asflush_document_to_packagealready does (157-1).regression_test.rs, and one Python test intest_core.pythat builds theissue's file with
zipfileonly. HLD 04 now says which bindings a storysplice checks, and that a splice refreshes the namespace facts.
rdocxarchive row is re-recorded.Part of #157. The reported failure is fixed. Python
add_picturenow succeedson the issue's file and on
fixture-report.docxfrom #158. What remains isthe
add_picturecolumn of the producer-traits matrix (157-4). It lands withthe acceptance matrices of #158 and #160. I ran that column locally on this
branch and every row passes, including "default namespace on the root" and
"inline content control on first runs".
Why
Python
Document.add_picturecallsinsert_picture_to_story. For the body,the story source is
self.document.to_xml(), which never writes a rootxmlns. The picture is spliced into that canonical XML and published throughset_story_source_xml. That function replaced the part and the typeddocument, but it kept the namespace facts of the part it had replaced. The
staged preparation then serializes the typed body again and parses the result
(
canonicalize_drawing_ids). A content control does not come back unchangedfrom that round trip, so the next flush saw a modified document. It checked
the stale facts, which still record
xmlns, against the spliced bytes, whichno longer declare it.
root_default_namespace_is_usedreturnsNonefor thatmismatch, and the check refused. Without a content control the typed document
survives the round trip, so the check never runs. That is why the issue needs
both traits, and why Google Docs exports hit it.
The refresh alone would have opened a silent failure. The canonical story
source drops the root default and every
w:bodydeclaration, and binds theroot
w,randmcprefixes to their canonical URIs (CT_Document::from_xmlnever records them in
extra_namespaces). The save path refuses all threecases for a modified document, but the splice path never ran that check. On
main, without a content control, every main-part splice on such a file isaccepted.
<producer/>underxmlns="urn:used-default"ends up in nonamespace,
<r:producer/>underxmlns:r="urn:not-relationships"ends up inthe relationships namespace, and
<x:producer/>inside a run under<w:body xmlns:x="urn:x">is written with an unbound prefix, so the part isnot namespace well-formed. With a content control, the picture and append
paths were refused only by the stale facts, and
insert_contentwas stillaccepted. That is why 157-3 lands with 157-1, and why it comes first.
Notes
accepts a splice that
mainrefuses. The check only adds refusals, andevery refusal the refresh lifts is either the reported false positive or a
case the check now refuses before publishing.
set_story_source_xml. I did not changeflush_document_to_packagetoderive its inputs from the part bytes, because that flush runs behind every
save. I did not route the body case through the typed body either, because
that would leave table-cell and text-box splices broken and change the
bytes of every Python
add_pictureoutput.source drops or rewrites: the default,
w,randmc. I considered thefull root branch of
unsafe_serializer_namespace_prefix, which typed savesuse, and took the narrower set. The canonical root replays the producer's
wp,a,pic,c,w14andw15values unchanged, and a spliceddrawing declares its own namespaces, so a splice cannot rebind those
prefixes. I checked this: with
xmlns:a="urn:not-a"on the root, a picturesplice writes
<a:graphic xmlns:a="...drawingml/2006/main">and the raw<a:producer/>keeps the producer binding. The full check would refusethose safe splices, which
mainaccepts.insert_html_fragmentinto the body already refuse them. Leaving them outwould also have been defensible. The canonical writer carries a
body declaration onto some retained elements, such as a direct raw child of
the body, but not onto a raw element nested in a paragraph or run. Review
found that the refresh would otherwise have accepted a splice beside a
content control on such a file and written
<x:foo/>with no binding inscope, where
mainrefused it through the stale facts. The cost is that asplice on a body-declared document whose output would have been correct
(a direct raw child, or an attribute on
w:porw:r) is now refused too.None of the Production readiness for editing real docx and pptx files: an acceptance contract, two realistic fixtures and two matrices #158 fixtures or matrix rows declares anything on
w:body. Ifsuch documents should keep accepting splices, the check would need to
classify where each body binding is used, which is a larger change for both
paths.
w,rormcto another URI, or declares a namespace onw:body, nowrefuses every story splice into the main part (
insert_content,import_fragment,remove_content_at,clone_content,move_content,insert_picture_to_story, and the appends ofappend_fragment_to_storysuch as
add_hyperlink_to_storyinto a table cell), with the error typededits already raise. On
mainthese splices were accepted and changed thenamespace of retained content, or left a prefix unbound, except the picture
and append paths beside a content control, which the stale facts refused.
insert_html_fragmentis not in this list. Into the body or a main-parttable cell it edits the typed body, and the typed flush already refused
these documents on
main. Its splice branch only serves headers andfooters.
under a root default namespace. On
main,insert_contentandinsert_picture_to_storywere accepted on such a file even with a contentcontrol in the body. The saved part then held an unprefixed
<sdt>under aroot with no default namespace, so the content control lost its namespace.
Such a file refused typed edits already. It now refuses splices too. If
these documents should accept edits, that is a change to the classification
of both paths, not to this one.
mainand are not touched by this change:
DocumentFragment::from_rangeslices the canonicalstory_sources()XMLwithout this check. Exporting a range from a document with a used root
default, then importing it, writes
<producer/>in no namespace. Thesame check could run in
from_rangebefore it slices, but that changes aread-only export API.
story_sourcesbuilds the main-part source withself.document.to_xml()and never runs
replay_nested_namespace_declarations, which the savepath does. With
<w:p xmlns:x="urn:x"><w:r><x:foo/>...in the body,insert_picture_to_storywrites<x:foo/>with no binding in scope,with or without a content control. Replaying the nested owners in the
story source, or refusing when an owner would be lost, is its own change.
under the indenting writer of
CT_Document::to_xml. It is why a contentcontrol makes the typed document look modified after a round trip. With
this PR the issue's file saves correctly, but whitespace inside
w:sdt,w:sdtPrandw:sdtContentstill grows on each rewrite. The fixdepends on the Producer traits: a matrix over every operation, and what still fails in it and around it #160 section 2 decision about the indenting writer.
_replace_document_bodygained an optionalroot_defaultargument.Tests
Added:
story_picture_splice_beside_content_control_ignores_unused_root_default(regression). It covers three cases: unused root default with a block
goog_rdk_0control, then the two controls (no default, no contentcontrol). Each case inserts a picture after the control (or after the
matching paragraph), appends a picture and a hyperlink to a table-cell
story, and appends a picture to the body. It asserts the exact body order
before and after a save and reopen, with the control's tag and text intact.
On
mainthe first case fails with the issue's error.rewritten_root_namespaces_block_story_splices_atomically(regression).Three roots:
<producer/>underxmlns="urn:used-default",<r:producer/>underxmlns:r="urn:not-relationships"and<mc:producer/>underxmlns:mc="urn:not-compatibility", each with andwithout a block control. A body picture, a table-cell picture and an
insert_contentall fail with the shadowed error for that prefix, and thesaved bytes are unchanged. On
mainevery case without a control succeedssilently, and
insert_contentsucceeds with a control too.body_declarations_and_a_rebound_root_w_block_story_splices_atomically(regression). Three documents, each with and without a block control:
<x:producer/>inside a run under<w:body xmlns:x="urn:x">, the sameelement as a direct child of the paragraph, and a
q:-prefixed documentwhose root binds
wtourn:produceraround a<w:producer/>. A bodypicture and an
insert_contentfail with shadowedxorw, and thesaved bytes are unchanged. I ran the same inputs against
origin/main:without a control both splices succeed and the saved part loses the
producer binding, and with a control the picture is refused while
insert_contentsucceeds.test_add_picture_beside_a_content_control_ignores_an_unused_root_default(
test_core.py). It covers theafter=form and the append form, with aparagraph after the control, and checks that the first picture lands
before that paragraph and the second after it. It fails on
mainwith theissue's error.
The test helpers use an
issue_157_prefix, so they do not read as F-157.Run on this branch:
cargo fmt --all --check: clean.cargo clippy -p rdocx --all-targets --all-features -- -D warningsand thesame for
rdocx-py: clean.cargo test -p rdocx --no-fail-fast:regression_test564 passed and 7ignored. The lib has 465 passed with 2 environment failures.
integration_testhas 314 passed with 5 environment failures. Doc-testspass.
cargo test -p rdocx-cli: passed.rewritten_root_namespaces_block_story_splices_atomicallyandbody_declarations_and_a_rebound_root_w_block_story_splices_atomically.cargo test -p rdocx-py, with the interpreter's library onDYLD_LIBRARY_PATH: passed.2 environment failures in
crates/rdocx-py/tests.mypy --strictontyping_smoke.pyand the package, andmypy.stubtest rdocx, are clean.python3 scripts/hash_harness.py --check: 49 entries match.python3 scripts/prose_check.py: 0 violations.python3 scripts/readme_doctests.py: passes after the re-record.add_picture okon both rows. Onfixture-report.docx,add_pictureplus save and reopen succeeds. Theadd_picturecolumn of the producer-traits matrix passes on every row.Environment-only failures, all pinned tool versions:
large_word_and_presentation_pdfs_preserve_logical_reading_orderand
word_and_powerpoint_chart_pixels_are_identical(pdftotext andrasterizer 26.01 pinned, 26.09 here).
integration_test:odt_reader_matches_pinned_libreoffice_structure,public_authored_theme_and_fonts_match_pinned_word_resolution,sanitized_public_authoring_fixture_passes_every_conformance_stage, andalso
f269_section_page_semantics::section_page_semantics_match_pinned_libreoffice_renderand
f267_table_style_conditional_tests::every_conditional_table_region_matches_word.The last two assert LibreOffice 26.2.5.2, and this machine has 26.8.0.3.
Both fail the same way on
origin/main(9a7ed71).test_rendering_threads.py, both tests, which pin pdfinfo 26.01.0.