From 7ac384839d9bf58873c301300cefd46f1afcf5fd Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 10:45:23 +0000 Subject: [PATCH] fix(docx): ignore malformed list levels --- crates/docx/src/read/props.rs | 25 ++++++++++++++++--------- crates/docx/src/read/styles.rs | 16 +++++++++++----- crates/docx/tests/it/passthrough.rs | 25 +++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/crates/docx/src/read/props.rs b/crates/docx/src/read/props.rs index 05386aa56..dc61b3a28 100644 --- a/crates/docx/src/read/props.rs +++ b/crates/docx/src/read/props.rs @@ -389,15 +389,22 @@ impl PropCtx { } "w:numPr" => { let num = k.child_val("w:numId").and_then(u32_of); - let ilvl = k.child_val("w:ilvl").and_then(u32_of); - let level = ilvl.unwrap_or(0).min(8) as u8; - match (num, ilvl) { - (Some(num), _) => p.numbering = Some(NumRef { num, level }), - // A level without a list: the list comes from the based-on style, as in - // a Heading 3 that only says "level 3" of Heading 2's list. Resolved once - // the styles are read (`resolve_list_levels`). - (None, Some(_)) => p.numbering = Some(NumRef { num: LEVEL_ONLY, level }), - (None, None) => {} + let ilvl_node = k.child("w:ilvl"); + let ilvl = + ilvl_node.and_then(|node| node.attr("w:val")).and_then(int).filter(|level| (0..=8).contains(level)).map(|level| level as u32); + // A missing level means level zero. A present but malformed/out-of-range + // level invalidates this numbering reference instead of silently becoming + // level zero; the latter can turn hostile input into a real list. + if ilvl_node.is_none() || ilvl.is_some() { + let level = ilvl.unwrap_or(0) as u8; + match (num, ilvl) { + (Some(num), _) => p.numbering = Some(NumRef { num, level }), + // A level without a list: the list comes from the based-on style, as in + // a Heading 3 that only says "level 3" of Heading 2's list. Resolved once + // the styles are read (`resolve_list_levels`). + (None, Some(_)) => p.numbering = Some(NumRef { num: LEVEL_ONLY, level }), + (None, None) => {} + } } } "w:pBdr" => p.borders = Some(self.borders(k)), diff --git a/crates/docx/src/read/styles.rs b/crates/docx/src/read/styles.rs index bba06907c..3d7e61bf8 100644 --- a/crates/docx/src/read/styles.rs +++ b/crates/docx/src/read/styles.rs @@ -6,6 +6,13 @@ use crate::units::tw_i16; use craft_wordmodel::props::PropChange; use std::sync::Arc; +fn numbering_level_index(value: Option<&str>) -> Option { + match value { + None => Some(0), + Some(value) => int(value).filter(|level| (0..=8).contains(level)).map(|level| level as usize), + } +} + /// Built-in style names Word stores in lower case, and how the UI shows them. pub(crate) const LOWER_NAMES: &[(&str, &str)] = &[ ("caption", "Caption"), @@ -294,10 +301,8 @@ impl Reader<'_> { let Some(id) = a.attr("w:abstractNumId").and_then(u32_of) else { continue }; let mut levels: Vec = (0..9).map(default_level).collect(); for l in a.children("w:lvl") { - let i = l.attr("w:ilvl").and_then(u32_of).unwrap_or(0) as usize; - if let Some(slot) = levels.get_mut(i) { - *slot = self.level(l, i); - } + let Some(i) = numbering_level_index(l.attr("w:ilvl")) else { continue }; + levels[i] = self.level(l, i); } if let Some(link) = a.child_val("w:numStyleLink") { links.push((id, link.to_string())); @@ -321,7 +326,8 @@ impl Reader<'_> { let (Some(id), Some(abs)) = (n.attr("w:numId").and_then(u32_of), n.child_val("w:abstractNumId").and_then(u32_of)) else { continue }; let (mut start_overrides, mut level_overrides) = (Vec::new(), Vec::new()); for o in n.children("w:lvlOverride").take(9) { - let lvl = o.attr("w:ilvl").and_then(u32_of).unwrap_or(0).min(8) as u8; + let Some(lvl) = numbering_level_index(o.attr("w:ilvl")) else { continue }; + let lvl = lvl as u8; if let Some(s) = o.child_val("w:startOverride").and_then(u32_of) { start_overrides.push((lvl, s)); } else if let Some(s) = o.child("w:lvl").and_then(|l| l.child_val("w:start")).and_then(u32_of) { diff --git a/crates/docx/tests/it/passthrough.rs b/crates/docx/tests/it/passthrough.rs index f59131e17..97741a813 100644 --- a/crates/docx/tests/it/passthrough.rs +++ b/crates/docx/tests/it/passthrough.rs @@ -320,6 +320,31 @@ fn list_identities_survive_and_a_copied_definition_gets_none() { assert_eq!(n.matches("1F2E3D4C").count(), 1, "{n}"); } +#[test] +fn malformed_numbering_levels_are_skipped_instead_of_becoming_level_zero() { + let numbering = r#""#; + let body = r#"default levelvalid levelnegative levelout of rangeoverflow"#; + let bytes = package(body, &[("rId1", &format!("{REL}/numbering"), "numbering.xml")], &[], &[("word/numbering.xml", numbering)]); + let doc = craft_docx::read(&bytes).unwrap(); + let paragraphs: Vec<_> = doc + .body + .iter() + .filter_map(|block| match &**block { + craft_wordmodel::Block::Para(p) => Some(p), + _ => None, + }) + .collect(); + assert_eq!(paragraphs.len(), 5); + assert_eq!(paragraphs[0].props.numbering.map(|n| (n.num, n.level)), Some((1, 0)), "an omitted level defaults to zero"); + assert_eq!(paragraphs[1].props.numbering.map(|n| (n.num, n.level)), Some((1, 2)), "a valid level is preserved"); + for p in ¶graphs[2..] { + assert_eq!(p.props.numbering, None, "malformed explicit level is not treated as a real list: {}", p.text); + } + let level_zero = doc.numbering.level(1, 0).expect("the valid level zero"); + assert_eq!(level_zero.format, craft_wordmodel::section::NumFormat::Decimal); + assert_eq!(level_zero.start, 1, "invalid level overrides do not affect level zero"); +} + #[test] fn extended_properties_are_kept_without_words_counts_and_personal_ones_follow_the_privacy_setting() { let app = r#"4210740000250000Microsoft Office Word020001700falseMFirmafalse290000falsehttps://example.com/false16.0000"#;