#6005: strict export checks metafile text; share note part names; count a chunk's unlisted pictures once

- Text in a WMF/EMF picture is shaped with the exporters' font resolution (shared metafile_text_run) and its missing glyphs join the MissingGlyphs report.
- The reader keeps one Arc<str> per source part name instead of a String per note or comment.
- A chunk imported twice adds its pictures past the list cap once (MissingPictures::add_unlisted).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FypgCX8G9hbs6XYZFsgjbL
This commit is contained in:
Martin Ball
2026-11-09 11:17:34 +00:00
co-authored by Claude Opus 5.5
parent 2c99020d1b
commit 40becaa3c2
14 changed files with 192 additions and 24 deletions
+1 -1
View File
@@ -191,7 +191,7 @@ story part it sits in such as `word/header2.xml`, and a reason code; `source_par
(`300 characters cannot be drawn (first 256 listed)`).
Each list stops at 256 entries separately; `truncated` is true when either was cut, and the totals
count every distinct character and picture found, those past the cap included. Only characters the output draws count (not hidden text, or deleted text
without `--markup`), and a character a fallback font draws is fine. A Private Use Area character
without `--markup`), and a character a fallback font draws is fine. Text inside a WMF or EMF picture counts too, shaped in the font the picture names with no fallback. A Private Use Area character
fails only if no installed font maps it. The result depends on the fonts installed on the host
(see #5624 for a fixed font set). An external picture target is never fetched: it is reported, not
loaded. Pictures dropped when a DOCX is opened (a missing part, an external target, or a `link only` picture with no embedded copy) are reported for DOCX input only. They have no page, so a PNG of any page is also refused for a missing or external picture anywhere in the document, unused headers included. `render --strict` checks only the page it
+71
View File
@@ -394,3 +394,74 @@ fn usage_errors_keep_their_line_breaks() {
let all = format!("{}{}", String::from_utf8_lossy(&r.stdout), stderr(&r));
assert!(all.lines().count() > 3 && all.lines().any(|l| l == "USAGE:"), "{all}");
}
/// A minimal EMF (88-byte header, 100 mm square frame) with one EMR_EXTTEXTOUTW record showing
/// `text` in the default font.
fn emf_with_text(text: &str) -> Vec<u8> {
let units: Vec<u16> = text.encode_utf16().collect();
let mut rec = vec![0u8; 76];
rec[0..4].copy_from_slice(&84u32.to_le_bytes());
rec[36..40].copy_from_slice(&10i32.to_le_bytes());
rec[40..44].copy_from_slice(&40i32.to_le_bytes());
rec[44..48].copy_from_slice(&(units.len() as u32).to_le_bytes());
rec[48..52].copy_from_slice(&76u32.to_le_bytes());
rec.extend(units.iter().flat_map(|u| u.to_le_bytes()));
while !rec.len().is_multiple_of(4) {
rec.push(0);
}
let len = rec.len() as u32;
rec[4..8].copy_from_slice(&len.to_le_bytes());
let eof: Vec<u8> = [14u32.to_le_bytes(), 20u32.to_le_bytes(), [0; 4], [0; 4], 20u32.to_le_bytes()].concat();
let total = 88 + rec.len() + eof.len();
let mut h = vec![0u8; 88];
h[0..4].copy_from_slice(&1u32.to_le_bytes());
h[4..8].copy_from_slice(&88u32.to_le_bytes());
h[8..24].copy_from_slice(&[0i32, 0, 100, 100].iter().flat_map(|v| v.to_le_bytes()).collect::<Vec<u8>>());
h[24..40].copy_from_slice(&[0i32, 0, 10000, 10000].iter().flat_map(|v| v.to_le_bytes()).collect::<Vec<u8>>());
h[40..44].copy_from_slice(&0x464D_4520u32.to_le_bytes());
h[44..48].copy_from_slice(&0x0001_0000u32.to_le_bytes());
h[48..52].copy_from_slice(&(total as u32).to_le_bytes());
h[52..56].copy_from_slice(&2u32.to_le_bytes());
h[56..58].copy_from_slice(&1u16.to_le_bytes());
h[72..76].copy_from_slice(&1000i32.to_le_bytes());
h[76..80].copy_from_slice(&1000i32.to_le_bytes());
h[80..84].copy_from_slice(&250i32.to_le_bytes());
h[84..88].copy_from_slice(&250i32.to_le_bytes());
[h, rec, eof].concat()
}
/// The text a metafile picture draws is checked like the page's own: a character no face covers
/// refuses the export from `convert` and `render`, naming the code point, and nothing is written.
#[test]
fn a_character_in_a_metafile_pictures_text_fails_strict_like_the_pages_own() {
if without_fonts() {
return;
}
let c = uncovered();
let cp = format!("U+{:04X}", c as u32);
let d = dir("emf-text");
let emf = emf_with_text(&format!("ab{c}"));
let input = d.join("in.docx");
std::fs::write(&input, docx_with_header_picture(&emf)).unwrap();
for out in ["out.pdf", "out.png"] {
let out = d.join(out);
let r = convert(&input, &out, true);
assert_eq!(r.status.code(), Some(3), "{}", stderr(&r));
assert_eq!(report_json(&r)["missing_glyphs"][0]["cp"], cp.as_str(), "{}", stderr(&r));
assert!(!out.exists(), "{} was written", out.display());
// Without --strict it converts, the character a hollow box.
let r = convert(&input, &out, false);
assert_eq!(r.status.code(), Some(0), "{}", stderr(&r));
std::fs::remove_file(&out).unwrap();
}
let out = d.join("p1.png");
let r = render(&input, &out, &["--strict"]);
assert_eq!(r.status.code(), Some(3), "{}", stderr(&r));
assert!(r.stdout.is_empty() && !out.exists());
assert_eq!(report_json(&r)["missing_glyphs"][0]["cp"], cp.as_str());
// Text the faces cover passes.
std::fs::write(&input, docx_with_header_picture(&emf_with_text("abc"))).unwrap();
let r = convert(&input, &d.join("ok.pdf"), true);
assert_eq!(r.status.code(), Some(0), "{}", stderr(&r));
std::fs::remove_dir_all(d).unwrap();
}
+13 -1
View File
@@ -81,7 +81,7 @@ pub struct MissingPictures {
/// A header, footer, note or comment story's part id ([`crate::Document::parts`]) to the
/// part it was read from (`word/header2.xml`). Ids freed and reused after opening may name
/// the old part.
pub part_names: std::collections::BTreeMap<u32, String>,
pub part_names: std::collections::BTreeMap<u32, std::sync::Arc<str>>,
/// A [`crate::Document::media`] key to the part its bytes were read from
/// (`word/media/image2.bin`).
pub media_parts: std::collections::BTreeMap<String, String>,
@@ -89,9 +89,21 @@ pub struct MissingPictures {
seen: std::collections::HashSet<MissingPicture>,
/// Hashes of the distinct pictures past the cap (8 bytes each), so the total counts them once.
unlisted: std::collections::HashSet<u64>,
/// The chunk parts whose unlisted pictures [`Self::add_unlisted`] has counted, so a chunk
/// imported twice counts them once.
counted_chunks: std::collections::HashSet<String>,
}
impl MissingPictures {
/// Add `n` pictures of the chunk part `chunk` that its reader counted but did not list. The
/// same chunk part counts once however often it is imported (its listed pictures dedupe
/// through [`Self::push`]).
pub fn add_unlisted(&mut self, chunk: &str, n: u64) {
if n > 0 && self.counted_chunks.insert(chunk.to_string()) {
self.total = self.total.saturating_add(n);
}
}
/// Add one (the same part, story and reason once).
pub fn push(&mut self, mut item: MissingPicture) {
for name in [&mut item.part, &mut item.story] {
+6 -6
View File
@@ -105,7 +105,7 @@ pub(crate) struct Reader<'p> {
/// The main document part's name.
main: String,
/// The part being read (`word/header2.xml`), for naming where a dropped picture was.
story: String,
story: Arc<str>,
/// The package's content types, read when first needed.
content_types: Option<ContentTypes>,
/// Parts kept for charts, diagrams and OLE objects (see `embed`).
@@ -186,7 +186,7 @@ fn read_opened_in(pkg: &Package, chunks: ChunkState) -> Result<(Document, usize,
glued: HashSet::new(),
shape_ids: false,
main: main.clone(),
story: main.clone(),
story: main.as_str().into(),
content_types: None,
embedded: EmbeddedManifest::default(),
txbx_links: Vec::new(),
@@ -235,18 +235,18 @@ fn read_opened_in(pkg: &Package, chunks: ChunkState) -> Result<(Document, usize,
let ex = part(rt::COMMENTS_EX);
let ids = part(rt::COMMENTS_IDS);
let cex = part(rt::COMMENTS_EXTENSIBLE);
r.story = p.clone();
r.story = p.as_str().into();
lenient("comments", r.read_comments(&p, ex.as_deref(), ids.as_deref(), cex.as_deref()));
}
if let Some(p) = part(rt::FOOTNOTES) {
r.story = p.clone();
r.story = p.as_str().into();
lenient("footnotes", r.read_notes(&p, true));
}
if let Some(p) = part(rt::ENDNOTES) {
r.story = p.clone();
r.story = p.as_str().into();
lenient("endnotes", r.read_notes(&p, false));
}
r.story = main.clone();
r.story = main.as_str().into();
// Bibliography sources live in a custom XML data store (`b:Sources`), related from the
// package root or the main part; Word's own files sometimes leave the relationship out, so
// the conventional `customXml/itemN.xml` names are tried too. Each part is read at most once.
+1 -1
View File
@@ -93,7 +93,7 @@ impl Reader<'_> {
// The nested records past its list are the file's too; their stories carry the chunk's
// part, so none is one the outer list already counted.
let unlisted = nested.total.saturating_sub(list_len);
self.doc.missing_pictures.total = self.doc.missing_pictures.total.saturating_add(unlisted);
self.doc.missing_pictures.add_unlisted(&part, unlisted);
self.doc.missing_pictures.truncated |= nested.truncated;
let mut blocks = self.doc.adopt_blocks(doc, keep, &mut self.next_perm);
// Bookmarks and comment marks read before the chunk stay before it.
+1 -1
View File
@@ -54,7 +54,7 @@ impl Reader<'_> {
if let Some(root) = root.filter(|r| r.name == "w:glossaryDocument" && ct.as_deref().is_none_or(|t| t == GLOSSARY_CONTENT_TYPE)) {
let rels = self.pkg.rels(&rel.target);
// Pictures in a building block belong to the glossary part, not the main story.
let outer = std::mem::replace(&mut self.story, rel.target.clone());
let outer = std::mem::replace(&mut self.story, rel.target.as_str().into());
entries = self.doc_parts(&root, &rels);
self.story = outer;
}
+4 -3
View File
@@ -128,7 +128,7 @@ impl Reader<'_> {
/// Remember a picture that was dropped at open, in the story being read.
pub(super) fn note_missing_picture(&mut self, part: &str, reason: MissingPictureReason) {
let part: String = part.chars().take(1024).collect();
let story = self.story.clone();
let story = self.story.to_string();
self.doc.missing_pictures.push(MissingPicture { part, story, reason });
}
@@ -153,7 +153,8 @@ impl Reader<'_> {
let kind = if footer { PartKind::Footer } else { PartKind::Header };
let id = self.doc.add_part(kind, Blocks::new());
self.hf_by_path.insert(path.to_string(), id);
self.doc.missing_pictures.part_names.insert(id, path.to_string());
let name: Arc<str> = path.into();
self.doc.missing_pictures.part_names.insert(id, Arc::clone(&name));
let root = match self.xml(path) {
Ok(Some(r)) => r,
_ => return Some(id),
@@ -163,7 +164,7 @@ impl Reader<'_> {
sc.in_hf = true;
let mut blocks = Blocks::new();
self.hf_depth += 1;
let outer = std::mem::replace(&mut self.story, path.to_string());
let outer = std::mem::replace(&mut self.story, name);
self.read_blocks(&mut sc, &root, &rels, &mut blocks, self.chunks.depth);
self.story = outer;
self.hf_depth = self.hf_depth.saturating_sub(1);
+10
View File
@@ -433,3 +433,13 @@ fn a_chunks_total_of_dropped_pictures_counts_past_the_list() {
assert!(d.missing_pictures.truncated);
assert_eq!(d.missing_pictures.total, n as u64);
}
/// The same chunk imported twice counts its unlisted pictures once, as it dedupes its listed ones.
#[test]
fn a_chunk_imported_twice_counts_its_dropped_pictures_past_the_list_once() {
let n = wordcraft_doc::MAX_MISSING_PICTURES + 44;
let inner = inner_with_dropped_pictures(n, false);
let d = read(&package(&format!("{}{CHUNK}{CHUNK}", p("Before")), Some(("docx", DOCX_MAIN, &inner)))).unwrap();
assert_eq!(d.missing_pictures.list.len(), wordcraft_doc::MAX_MISSING_PICTURES);
assert_eq!(d.missing_pictures.total, n as u64);
}
+15 -1
View File
@@ -98,7 +98,7 @@ fn a_header_pictures_story_is_the_header_part() {
let d = wordcraft_docx::read(&bytes).unwrap();
assert_eq!(missing(&d), [("https://example.invalid/logo.png".into(), "word/header2.xml".into(), MissingPictureReason::ExternalTarget)]);
assert_eq!(d.missing_pictures.main, "word/document.xml");
assert!(d.missing_pictures.part_names.values().any(|n| n == "word/header2.xml"));
assert!(d.missing_pictures.part_names.values().any(|n| &**n == "word/header2.xml"));
assert_eq!(d.missing_pictures.media_parts.get("ok.png").map(String::as_str), Some("word/media/ok.png"));
}
@@ -147,3 +147,17 @@ fn the_list_is_capped_and_the_document_is_still_equal() {
assert_eq!(d, e);
assert!(!serde_json::to_string(&d).unwrap().contains("gone"));
}
/// Every note of a part shares the one allocation of its source name, so a file with many notes
/// in a part with a long name does not keep a copy of the name per note.
#[test]
fn the_notes_of_a_part_share_one_copy_of_its_name() {
let notes: String = (1..=3).map(|i| format!(r#"<w:footnote w:id="{i}"><w:p><w:r><w:t>n{i}</w:t></w:r></w:p></w:footnote>"#)).collect();
let rels = format!(r#"<Relationship Id="rIdF" Type="{REL}/footnotes" Target="footnotes.xml"/>"#);
let part = format!(r#"<w:footnotes {NS}>{notes}</w:footnotes>"#);
let d = wordcraft_docx::read(&package("x", &rels, None, &[("word/footnotes.xml", part.into_bytes())])).unwrap();
let names: Vec<_> = d.missing_pictures.part_names.values().collect();
assert!(names.len() >= 3, "{names:?}");
assert!(names.iter().all(|n| &***n == "word/footnotes.xml"));
assert!(names.windows(2).all(|w| std::sync::Arc::ptr_eq(w[0], w[1])), "each note holds its own copy of the part name");
}
+1 -1
View File
@@ -84,7 +84,7 @@ use headers::MAX_HEADER_ITEMS;
#[cfg(test)]
use headers::placed_cost;
use headers::placed_cost_by;
pub use missing::{MAX_MISSING_GLYPHS, MissingGlyphs, missing_watermark_glyphs, watermark_run};
pub use missing::{MAX_MISSING_GLYPHS, MissingGlyphs, metafile_text_run, missing_watermark_glyphs, watermark_run};
pub use notes::DECOR_STORY;
pub(crate) use notes::NOTE_SEP_W;
use notes::{MAX_NOTE_PASSES, footnote_pages};
+66 -4
View File
@@ -2,7 +2,7 @@
//! id 0 in a [`Draw::Glyphs`] of the display list, traced back to its character through the
//! run's `text` and per-glyph `ranges`. Backs strict export (#6005).
use std::collections::BTreeMap;
use std::collections::{BTreeMap, HashMap};
use std::ops::{Range, RangeInclusive};
use wordcraft_doc::Document;
@@ -121,7 +121,11 @@ impl DocLayout {
continue;
}
for d in page_display(doc, page, opts) {
found.walk(&d);
found.walk(doc, &d);
}
// The page picture is drawn behind every page, outside the display list.
if let Some(pic) = &doc.settings.page_picture {
found.metafile_text(doc, &pic.media);
}
}
found.finish()
@@ -143,6 +147,16 @@ pub fn watermark_run(wm: &wordcraft_doc::Watermark, max_chars: usize) -> Option<
Some((face, text, shaped))
}
/// How a metafile text run is shaped for output, as the PDF export and the page raster both do:
/// in the face its font name resolves to (no fallback face), so a character that face lacks shapes
/// to glyph id 0. Strict export checks these same glyphs, so the check and the exporters can't
/// drift.
pub fn metafile_text_run(t: &wordcraft_metafile::Text) -> (wordcraft_fonts::FaceRef, Vec<ShapedGlyph>) {
let face = wordcraft_fonts::word::resolve(&t.font, t.bold, t.italic).face;
let shaped = wordcraft_fonts::shape(&face, &t.text, &[], |c| c);
(face, shaped)
}
/// The characters of the watermark that draw as an empty box (glyph id 0), counted as
/// [`DocLayout::missing_glyphs`] counts them; sorted by code point. The watermark is on every
/// page, so it counts once.
@@ -157,24 +171,49 @@ pub fn missing_watermark_glyphs(wm: &wordcraft_doc::Watermark, max_chars: usize)
#[derive(Default)]
struct Found {
/// The text runs of each metafile picture met, by media key, shaped once: (face, text, ranges, glyph ids).
metafiles: HashMap<String, Vec<MetaRun>>,
counts: BTreeMap<char, u64>,
truncated: bool,
/// Every distinct code point, started when the list passes its cap.
over: Option<Box<CodePointSet>>,
}
/// A metafile text run as drawn: its face, text, per-glyph ranges and glyph ids.
struct MetaRun {
face: wordcraft_fonts::FaceRef,
text: String,
ranges: Vec<Range<usize>>,
gids: Vec<u32>,
}
impl Found {
/// The text of the metafile picture `key` (nothing for a picture that is not a metafile, does
/// not parse or has no size: the exporters draw none of it), counted once per call.
fn metafile_text(&mut self, doc: &Document, key: &str) {
if !self.metafiles.contains_key(key) {
let runs = doc.media.get(key).map(|bytes| meta_runs(bytes)).unwrap_or_default();
self.metafiles.insert(key.to_string(), runs);
}
let Some(runs) = self.metafiles.remove(key) else { return };
for r in &runs {
self.run(&r.face, r.gids.iter().copied(), &r.text, &r.ranges);
}
self.metafiles.insert(key.to_string(), runs);
}
fn finish(self) -> MissingGlyphs {
let total = self.over.as_ref().map_or(self.counts.len(), |s| s.len());
MissingGlyphs { chars: self.counts.into_iter().collect(), truncated: self.truncated, total, seen: self.over }
}
fn walk(&mut self, d: &Draw) {
fn walk(&mut self, doc: &Document, d: &Draw) {
match d {
Draw::Glyphs { face, glyphs, text, ranges, .. } => self.run(face, glyphs.iter().map(|g| g.0), text, ranges),
Draw::Image { media, .. } => self.metafile_text(doc, media),
Draw::Figure { draws: items, .. } | Draw::Turned { items, .. } | Draw::Rotated { items, .. } | Draw::Clip { items, .. } => {
for d in items {
self.walk(d);
self.walk(doc, d);
}
}
_ => {}
@@ -224,6 +263,29 @@ impl Found {
}
}
/// The text runs of a metafile picture that the exporters draw (a run whose layout fails is not).
fn meta_runs(bytes: &[u8]) -> Vec<MetaRun> {
if !wordcraft_metafile::is_metafile(bytes) {
return Vec::new();
}
let Ok(pic) = wordcraft_metafile::parse(bytes) else { return Vec::new() };
if !pic.has_size() {
return Vec::new();
}
let mut out = Vec::new();
for item in &pic.items {
let wordcraft_metafile::Item::Text(t) = item else { continue };
let (face, shaped) = metafile_text_run(t);
let glyphs: Vec<_> = shaped.iter().map(|g| (g.cluster, f64::from(g.x_advance), f64::from(g.x_offset), f64::from(g.y_offset))).collect();
if t.layout(face.upem, face.ascent, face.descent, &glyphs).is_none() {
continue;
}
let ranges = cluster_ranges(&t.text, &shaped);
out.push(MetaRun { gids: shaped.iter().map(|g| g.gid).collect(), text: t.text.clone(), ranges, face });
}
out
}
/// The byte range of `text` each of `shaped` shows: from its cluster to the next cluster start in
/// the text (or its end), whichever order the glyphs are in. A ligature's glyph covers all its
/// characters; the glyphs of one cluster share its range. Ranges always lie on character
+1 -1
View File
@@ -163,7 +163,7 @@ fn collect_media(draws: &[Draw], out: &mut Vec<String>) {
/// The story a placed item is in, named as [`undecodable_pictures`] says.
fn story_name(doc: &Document, region: Region, it: &Placed) -> String {
let names = &doc.missing_pictures;
let known = |id: Option<u32>| id.and_then(|id| names.part_names.get(&id)).cloned();
let known = |id: Option<u32>| id.and_then(|id| names.part_names.get(&id)).map(|n| n.to_string());
match region {
Region::Header(id) => known(id).unwrap_or_else(|| "header".into()),
Region::Footer(id) => known(id).unwrap_or_else(|| "footer".into()),
+1 -2
View File
@@ -183,8 +183,7 @@ impl Exporter<'_> {
/// Draws a metafile text run as PDF text (outlines when the font cannot be embedded). `tf` maps
/// the run's own space (see [`PlacedItem::Text`]) to the page.
pub(crate) fn vector_text(&mut self, s: &mut Surface, t: &wordcraft_metafile::Text, tf: kurbo::Affine) {
let face = wordcraft_fonts::word::resolve(&t.font, t.bold, t.italic).face;
let shaped = wordcraft_fonts::shape(&face, &t.text, &[], |c| c);
let (face, shaped) = wordcraft_layout::metafile_text_run(t);
let glyphs: Vec<_> = shaped.iter().map(|g| (g.cluster, f64::from(g.x_advance), f64::from(g.x_offset), f64::from(g.y_offset))).collect();
let Some(lay) = t.layout(face.upem, face.ascent, face.descent, &glyphs) else { return };
let font = self.font(&face);
+1 -2
View File
@@ -254,8 +254,7 @@ pub(crate) fn draw_vector(ctx: &mut RenderContext, v: &Vector, r: &kurbo::Rect,
/// Draws a metafile text run: `tf` maps the run's own space (see [`PlacedItem::Text`]) to the device.
fn draw_vector_text(ctx: &mut RenderContext, t: &wordcraft_metafile::Text, tf: Affine) {
let face = wordcraft_fonts::word::resolve(&t.font, t.bold, t.italic).face;
let shaped = wordcraft_fonts::shape(&face, &t.text, &[], |c| c);
let (face, shaped) = wordcraft_layout::metafile_text_run(t);
let glyphs: Vec<_> = shaped.iter().map(|g| (g.cluster, f64::from(g.x_advance), f64::from(g.x_offset), f64::from(g.y_offset))).collect();
let Some(lay) = t.layout(face.upem, face.ascent, face.descent, &glyphs) else { return };
let k = lay.em / face.upem.max(1.0);