From 64b0447edb35c9f052eac5939a8dece1ad7b4b70 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:08:16 +0200 Subject: [PATCH 1/3] Refuse a story splice that would drop or rebind a namespace 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 under xmlns="urn:used-default", or under a non-canonical xmlns:r, silently changed namespace, and an nested in a run under 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 #157. --- crates/rdocx/src/document.rs | 22 +++++ crates/rdocx/tests/regression_test.rs | 134 ++++++++++++++++++++++++++ docs/hld/04-opc-and-packaging.md | 11 ++- 3 files changed, 164 insertions(+), 3 deletions(-) diff --git a/crates/rdocx/src/document.rs b/crates/rdocx/src/document.rs index 5c4b2b46..b7372432 100644 --- a/crates/rdocx/src/document.rs +++ b/crates/rdocx/src/document.rs @@ -6594,6 +6594,28 @@ fn restart_merges_continued_below(table: &mut CT_Tbl, row_index: usize) -> Resul fn set_story_source_xml(document: &mut Document, part_name: &str, xml: Vec) -> Result<()> { if part_name == document.doc_part_name { + // The main-part story source is canonical XML. It drops a root + // default namespace and every body declaration, and binds the root + // `w`, `r` and `mc` prefixes to their canonical URIs. The save path's + // check on those declarations keeps retained content of the replaced + // part in its namespace. + let rewritten_root_declarations: Vec<_> = document + .root_namespace_declarations + .iter() + .filter(|(name, _)| { + matches!(name.as_str(), "xmlns" | "xmlns:w" | "xmlns:r" | "xmlns:mc") + }) + .cloned() + .collect(); + if let Some(prefix) = unsafe_serializer_namespace_prefix( + &rewritten_root_declarations, + &document.body_namespace_declarations, + document.package.get_part(part_name), + ) { + return Err(Error::Other(format!( + "cannot serialize a modified document with a shadowed `{prefix}` namespace" + ))); + } document.document = CT_Document::from_xml(&xml)?; document.package.set_part(part_name, xml); } else if document.comments_part_name.as_deref() == Some(part_name) { diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index d143b3c7..ee092d0e 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -13522,6 +13522,140 @@ fn used_root_default_namespace_still_fails_atomically() { assert_eq!(document.to_bytes().unwrap(), before); } +/// A Google Docs shaped main part: extra root namespace declarations, an +/// optional `goog_rdk_0` block content control and a one-cell table. +fn issue_157_document(root_declarations: &str, content_control: bool, producer: &str) -> Document { + let inside = "Inside the control."; + let inside = if content_control { + format!( + r#"{inside}"# + ) + } else { + inside.to_owned() + }; + document_with_content_controls(&format!( + r#"{producer}Before the control.{inside}cell"# + )) +} + +fn issue_157_insert_picture( + document: &mut Document, + story: &StoryId, + after: Option<&ContentLocation>, +) -> rdocx::Result { + document.insert_picture_to_story( + story, + after, + b"issue 157 image payload", + "issue_157.png", + Some(Length::pt(12.0)), + Some(Length::pt(8.0)), + ) +} + +#[test] +fn rewritten_root_namespaces_block_story_splices_atomically() { + // The canonical main-part source of a story splice drops the root + // default and rebinds the root `w`, `r` and `mc` prefixes, so retained + // content that uses one of those root bindings would silently change + // namespace. + let roots = [ + (r#" xmlns="urn:used-default""#, "", "default"), + (r#" xmlns:r="urn:not-relationships""#, "", "r"), + ( + r#" xmlns:mc="urn:not-compatibility""#, + "", + "mc", + ), + ]; + for (root_declarations, producer, prefix) in roots { + for content_control in [false, true] { + let case = format!("{prefix}, content control {content_control}"); + let mut document = issue_157_document(root_declarations, content_control, producer); + let before = document.to_bytes().unwrap(); + let body = f254_story(&document, StoryKind::Body); + let cell = f254_story(&document, StoryKind::TableCell); + let errors = [ + issue_157_insert_picture(&mut document, &body, None).unwrap_err(), + issue_157_insert_picture(&mut document, &cell, None).unwrap_err(), + document + .insert_content(&ContentLocation::end(body), f254_paragraph("spliced")) + .unwrap_err(), + ]; + for error in errors { + assert!( + error + .to_string() + .contains(&format!("shadowed `{prefix}` namespace")), + "{case}: {error}" + ); + } + assert_eq!(document.to_bytes().unwrap(), before, "{case}"); + } + } +} + +#[test] +fn body_declarations_and_a_rebound_root_w_block_story_splices_atomically() { + // The canonical main-part source drops every declaration on `w:body`, so + // a raw element nested in a paragraph or run would keep an unbound + // prefix. It also binds the root `w` prefix to WordprocessingML, so a + // producer element under another root `w` binding would change namespace. + for content_control in [false, true] { + let control = |q: &str| { + if content_control { + format!( + r#"<{q}:sdt><{q}:sdtPr><{q}:tag {q}:val="goog_rdk_0"/><{q}:sdtContent><{q}:p><{q}:r><{q}:t>Inside the control."# + ) + } else { + String::new() + } + }; + let (w_control, q_control) = (control("w"), control("q")); + let documents = [ + ( + format!( + r#"run{w_control}"# + ), + "x", + ), + ( + format!( + r#"run{w_control}"# + ), + "x", + ), + ( + format!( + r#"run{q_control}"# + ), + "w", + ), + ]; + for (xml, prefix) in documents { + let case = format!("content control {content_control}: {xml}"); + let mut document = document_with_content_controls(&xml); + let before = document.to_bytes().unwrap(); + let body = f254_story(&document, StoryKind::Body); + let errors = [ + issue_157_insert_picture(&mut document, &body, None).unwrap_err(), + document + .insert_content(&ContentLocation::end(body), f254_paragraph("spliced")) + .unwrap_err(), + ]; + for error in errors { + assert!( + error + .to_string() + .contains(&format!("shadowed `{prefix}` namespace")), + "{case}: {error}" + ); + } + assert_eq!(document.to_bytes().unwrap(), before, "{case}"); + } + } +} + #[test] fn try_replace_text_publishes_only_a_preflighted_candidate() { let mut document = Document::new(); diff --git a/docs/hld/04-opc-and-packaging.md b/docs/hld/04-opc-and-packaging.md index b9c29769..e3819778 100644 --- a/docs/hld/04-opc-and-packaging.md +++ b/docs/hld/04-opc-and-packaging.md @@ -453,9 +453,14 @@ default may be omitted without blocking a typed mutation. An unprefixed element that inherits it keeps the binding live and blocks modified serialization. Nested default declarations shadow the root declaration, including when they repeat the same URI, and unprefixed attributes never use a default namespace. -Malformed or ambiguous declarations fail closed. After a successful canonical -publication, the document refreshes its root and body namespace facts from the -published main-story bytes so a later save applies the same classification. +Malformed or ambiguous declarations fail closed. A story splice into the main +part also publishes canonical XML, so it applies the same root default +classification before it publishes. Like a modified save, it also refuses any +declaration on `w:body`, which the canonical body drops, and a root `w`, `r` or +`mc` declaration bound to another URI, because the canonical root rebinds those +prefixes. After a successful canonical publication, the document refreshes its +root and body namespace facts from the published main-story bytes so a later +save applies the same classification. Paragraph line spacing retains the signed integer path required by WordprocessingML and accepts one bounded producer deviation. A plain signed From 898fdd65e143f0094bfe6fb4c815fb67ce393597 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:08:45 +0200 Subject: [PATCH 2/3] Refresh main-part namespace facts after a story splice 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 #157. --- crates/rdocx-py/tests/test_core.py | 42 ++++++++++- crates/rdocx/src/document.rs | 4 + crates/rdocx/tests/regression_test.rs | 102 ++++++++++++++++++++++++++ docs/hld/04-opc-and-packaging.md | 6 +- 4 files changed, 150 insertions(+), 4 deletions(-) diff --git a/crates/rdocx-py/tests/test_core.py b/crates/rdocx-py/tests/test_core.py index 0f8ce927..380880e0 100644 --- a/crates/rdocx-py/tests/test_core.py +++ b/crates/rdocx-py/tests/test_core.py @@ -9,7 +9,7 @@ import pytest -def _replace_document_body(document, body): +def _replace_document_body(document, body, root_default=None): source = io.BytesIO(document.to_bytes()) result = io.BytesIO() with zipfile.ZipFile(source) as source_zip: @@ -20,6 +20,9 @@ def _replace_document_body(document, body): start = data.index(b"") + len(b"") end = data.index(b"") data = data[:start] + body.encode() + data[end:] + if root_default: + root = f'Before the control." + '' + "Inside the control." + "" + "After the control." + ) + tasks = "http://schemas.microsoft.com/office/tasks/2019/documenttasks" + for root_default in (None, tasks): + document = _replace_document_body(rdocx.Document(), body, root_default) + control = next( + item + for item in document.story_items + if item.story.kind == "body" and item.kind == "content_control" + ) + size = {"width": rdocx.Inches(1), "height": rdocx.Inches(1)} + after = document.add_picture(_one_pixel_png(), "x.png", after=control, **size) + appended = document.add_picture(_one_pixel_png(), "x.png", **size) + assert after.kind == appended.kind == "paragraph" + + xml = _document_xml(rdocx.Document.from_bytes(document.to_bytes())) + assert xml.count(b"") == 2 + positions = [ + xml.index(b"Before the control."), + xml.index(b''), + xml.index(b"Inside the control."), + xml.index(b""), + xml.index(b""), + xml.index(b"After the control."), + xml.rindex(b""), + ] + assert positions == sorted(positions) + + def _relationship_target(document, part_name, relationship_id): owner = part_name.lstrip("/") directory, filename = posixpath.split(owner) diff --git a/crates/rdocx/src/document.rs b/crates/rdocx/src/document.rs index b7372432..88c60ceb 100644 --- a/crates/rdocx/src/document.rs +++ b/crates/rdocx/src/document.rs @@ -6616,8 +6616,12 @@ fn set_story_source_xml(document: &mut Document, part_name: &str, xml: Vec) "cannot serialize a modified document with a shadowed `{prefix}` namespace" ))); } + let namespace_scopes = document_namespace_scopes(&xml)?; document.document = CT_Document::from_xml(&xml)?; document.package.set_part(part_name, xml); + document.root_namespace_declarations = namespace_scopes.root_declarations; + document.body_namespace_declarations = namespace_scopes.body_declarations; + document.body_namespace_bindings = namespace_scopes.body_bindings; } else if document.comments_part_name.as_deref() == Some(part_name) { document.comments = Some(rdocx_oxml::comments::CT_Comments::from_xml(&xml)?); document.package.set_part(part_name, xml); diff --git a/crates/rdocx/tests/regression_test.rs b/crates/rdocx/tests/regression_test.rs index ee092d0e..c0644702 100644 --- a/crates/rdocx/tests/regression_test.rs +++ b/crates/rdocx/tests/regression_test.rs @@ -13522,6 +13522,9 @@ fn used_root_default_namespace_still_fails_atomically() { assert_eq!(document.to_bytes().unwrap(), before); } +const ISSUE_157_UNUSED_ROOT_DEFAULT: &str = + r#" xmlns="http://schemas.microsoft.com/office/tasks/2019/documenttasks""#; + /// A Google Docs shaped main part: extra root namespace declarations, an /// optional `goog_rdk_0` block content control and a one-cell table. fn issue_157_document(root_declarations: &str, content_control: bool, producer: &str) -> Document { @@ -13538,6 +13541,43 @@ fn issue_157_document(root_declarations: &str, content_control: bool, producer: )) } +fn issue_157_paragraph_summary(paragraph: &ParagraphRef<'_>) -> String { + let has_picture = paragraph.runs().any(|run| { + run.items() + .any(|item| matches!(item, RunItemRef::Drawing(drawing) if drawing.is_inline())) + }); + if has_picture { + "picture".to_owned() + } else { + format!("p:{}", paragraph.text()) + } +} + +fn issue_157_body_summary(document: &Document) -> Vec { + document + .body_items() + .map(|item| match item { + BodyItemRef::Paragraph(paragraph) => issue_157_paragraph_summary(¶graph), + BodyItemRef::ContentControl(control) => format!( + "sdt:{}:{}", + control.tag().unwrap_or_default(), + control.text() + ), + BodyItemRef::Table(table) => format!( + "table:{}", + table + .cell(0, 0) + .unwrap() + .paragraphs() + .map(|paragraph| issue_157_paragraph_summary(¶graph)) + .collect::>() + .join("|") + ), + BodyItemRef::UnsupportedXml(raw) => format!("raw:{}", String::from_utf8_lossy(raw)), + }) + .collect() +} + fn issue_157_insert_picture( document: &mut Document, story: &StoryId, @@ -13553,6 +13593,68 @@ fn issue_157_insert_picture( ) } +#[test] +fn story_picture_splice_beside_content_control_ignores_unused_root_default() { + // A story splice publishes canonical main-part XML without the unused + // root default. The flush that follows must classify those bytes, not + // the declarations of the part they replaced. + for (root_default, content_control) in [(true, true), (false, true), (true, false)] { + let case = format!("root default {root_default}, content control {content_control}"); + let root_declarations = if root_default { + ISSUE_157_UNUSED_ROOT_DEFAULT + } else { + "" + }; + let mut document = issue_157_document(root_declarations, content_control, ""); + let body = f254_story(&document, StoryKind::Body); + let anchor = document + .story_items(&body) + .unwrap() + .into_iter() + .find(|item| { + if content_control { + item.kind() == StoryItemKind::ContentControl + } else { + item.text().unwrap().as_deref() == Some("Inside the control.") + } + }) + .unwrap() + .location() + .clone(); + let after = issue_157_insert_picture(&mut document, &body, Some(&anchor)) + .unwrap_or_else(|error| panic!("{case}: picture after the anchor: {error}")); + assert_eq!(after.item_kind(), StoryItemKind::Paragraph, "{case}"); + + let cell = f254_story(&document, StoryKind::TableCell); + issue_157_insert_picture(&mut document, &cell, None) + .unwrap_or_else(|error| panic!("{case}: picture in the table cell: {error}")); + let cell = f254_story(&document, StoryKind::TableCell); + document + .add_hyperlink_to_story(&cell, "link", "https://example.invalid/issue-157") + .unwrap_or_else(|error| panic!("{case}: hyperlink in the table cell: {error}")); + let body = f254_story(&document, StoryKind::Body); + issue_157_insert_picture(&mut document, &body, None) + .unwrap_or_else(|error| panic!("{case}: appended picture: {error}")); + + let anchor = if content_control { + "sdt:goog_rdk_0:Inside the control." + } else { + "p:Inside the control." + }; + let expected = [ + "p:Before the control.", + anchor, + "picture", + "table:p:cell|picture|p:link", + "picture", + ]; + assert_eq!(issue_157_body_summary(&document), expected, "{case}"); + let saved = document.to_bytes().unwrap(); + let reopened = Document::from_bytes(&saved).unwrap(); + assert_eq!(issue_157_body_summary(&reopened), expected, "{case}"); + } +} + #[test] fn rewritten_root_namespaces_block_story_splices_atomically() { // The canonical main-part source of a story splice drops the root diff --git a/docs/hld/04-opc-and-packaging.md b/docs/hld/04-opc-and-packaging.md index e3819778..c75aff10 100644 --- a/docs/hld/04-opc-and-packaging.md +++ b/docs/hld/04-opc-and-packaging.md @@ -458,9 +458,9 @@ part also publishes canonical XML, so it applies the same root default classification before it publishes. Like a modified save, it also refuses any declaration on `w:body`, which the canonical body drops, and a root `w`, `r` or `mc` declaration bound to another URI, because the canonical root rebinds those -prefixes. After a successful canonical publication, the document refreshes its -root and body namespace facts from the published main-story bytes so a later -save applies the same classification. +prefixes. After a successful canonical publication, by a save or by a story +splice, the document refreshes its root and body namespace facts from the +published main-story bytes so a later save applies the same classification. Paragraph line spacing retains the signed integer path required by WordprocessingML and accepts one bounded producer deviation. A plain signed From 46c128e3c94688374b297c046222751e9dd1e658 Mon Sep 17 00:00:00 2001 From: Hadrien Mary Date: Sun, 27 Sep 2026 17:11:16 +0200 Subject: [PATCH 3/3] Re-record the rdocx archive measurement 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 #157. --- README.md | 2 +- scripts/readme_doctests.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 479b462e..4427c079 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ rows are the enforced release-mode bounds plus one dated observation. | Measurement | Value | Version | Platform | Build mode | Input | Command | Statistic | Measured on | |---|---|---|---|---|---|---|---|---| -| Crates.io archive: rdocx | 1,092,256 compressed bytes, 6,498,484 member bytes, 36 members | 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rdocx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | +| Crates.io archive: rdocx | 1,094,567 compressed bytes, 6,509,647 member bytes, 36 members | 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | `cargo package --locked --no-verify` | Tracked `rdocx` package inventory | `python3 scripts/readme_doctests.py --record-measurements` | gzip archive bytes, tar member bytes, tar member count | 2026-09-26 | | Large-document layout throughput | minimum 250 pages/s, observed 31,019.1 pages/s | rdocx 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | release, one test thread | 1,000 one-page paragraphs with deterministic fonts | `cargo test -p rdocx --test regression_test --release a_thousand_page_document_paginates_and_renders_within_the_declared_limits -- --ignored --exact --nocapture --test-threads=1` | pages per wall-clock second | 2026-09-19 | | Large-document layout peak allocation | maximum 64 MiB, observed 29.03 MiB | rdocx 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | release, one test thread | 1,000 one-page paragraphs with deterministic fonts | `cargo test -p rdocx --test regression_test --release a_thousand_page_document_paginates_and_renders_within_the_declared_limits -- --ignored --exact --nocapture --test-threads=1` | peak live allocation | 2026-09-19 | | Large-document PDF throughput | minimum 1,000 pages/s, observed 60,058.0 pages/s | rdocx 0.14.0 | macOS 26.6.2, Apple M5 Max, arm64 | release, one test thread | 1,000 deterministic layout pages | `cargo test -p rdocx --test regression_test --release a_thousand_page_document_paginates_and_renders_within_the_declared_limits -- --ignored --exact --nocapture --test-threads=1` | pages per wall-clock second | 2026-09-19 | diff --git a/scripts/readme_doctests.py b/scripts/readme_doctests.py index e24c643b..a56cf5b3 100644 --- a/scripts/readme_doctests.py +++ b/scripts/readme_doctests.py @@ -383,7 +383,7 @@ class ReadmeCase: "oxml-opc": (92_122, 355_510, 12), "oxml-pdf": (66_015, 304_432, 14), "oxml-sml": (12_511, 49_803, 6), - "rdocx": (1_092_256, 6_498_484, 36), + "rdocx": (1_094_567, 6_509_647, 36), "rdocx-cli": (33_805, 145_256, 8), "rdocx-html": (15_486, 63_894, 11), "rdocx-layout": (255_752, 1_385_701, 15),