diff --git a/crates/xlsx/src/passthrough.rs b/crates/xlsx/src/passthrough.rs index 187009b..f121977 100644 --- a/crates/xlsx/src/passthrough.rs +++ b/crates/xlsx/src/passthrough.rs @@ -32,6 +32,7 @@ use quick_xml::events::{BytesStart, Event}; use quick_xml::name::QName; use quick_xml::{Reader, Writer}; +use crate::IoError; use crate::package::{self, normalize}; use crate::read::Ctx; use crate::write::{NS_REL, Out, Rels, XML_DECL}; @@ -54,6 +55,23 @@ const MAX_FRAGMENT: usize = 4 * 1024 * 1024; /// Deepest element nesting accepted in a kept fragment. const MAX_FRAGMENT_DEPTH: usize = 256; +#[cfg(test)] +thread_local! { + /// A smaller kept-bytes allowance for tests on this thread. + pub(crate) static KEPT_TOTAL_FOR_TEST: std::cell::Cell = const { std::cell::Cell::new(MAX_KEPT_TOTAL) }; +} + +fn kept_total() -> usize { + #[cfg(test)] + { + KEPT_TOTAL_FOR_TEST.with(std::cell::Cell::get) + } + #[cfg(not(test))] + { + MAX_KEPT_TOTAL + } +} + const NS_REL_STRICT: &str = "http://purl.oclc.org/ooxml/officeDocument/relationships"; const CT_VML: &str = "application/vnd.openxmlformats-officedocument.vmlDrawing"; @@ -194,7 +212,7 @@ impl KeptReader { strict: false, defaults: HashMap::new(), overrides: HashMap::new(), - budget: MAX_KEPT_TOTAL, + budget: kept_total(), parts_left: MAX_KEPT_PARTS, }; if let Ok(root) = raw_rels(cx, "") @@ -446,11 +464,19 @@ impl KeptReader { seen: &mut HashSet, ) -> Result<(PreservedPart, Vec), String> { let rel = rid.and_then(|id| rels.iter().find(|r| r.id == id && !r.external)).ok_or("the sheet has no VML drawing")?; - let data = match cx.pkg.read_limited(&rel.target, MAX_META) { + // Charged to the kept total like any other part: many sheets can point at one highly + // compressed VML drawing, and each keeps its own copy. + if self.budget == 0 { + return Err("it is too large".into()); + } + let limit = MAX_META.min(u64::try_from(self.budget).unwrap_or(u64::MAX)); + let data = match cx.pkg.read_limited(&rel.target, limit) { Ok(Some(d)) => d, Ok(None) => return Err(format!("{} is missing", rel.target)), + Err(IoError::TooLarge(_)) => return Err("it is too large".into()), Err(e) => return Err(e.to_string()), }; + self.budget = self.budget.saturating_sub(data.len()); let text = String::from_utf8(data).map_err(|_| "the VML drawing is not UTF-8".to_string())?; let text = strip_notes(&text); let vrels = raw_rels(cx, &rel.target)?; diff --git a/crates/xlsx/src/tests/passthrough_tests.rs b/crates/xlsx/src/tests/passthrough_tests.rs index 10b2681..4cde9d6 100644 --- a/crates/xlsx/src/tests/passthrough_tests.rs +++ b/crates/xlsx/src/tests/passthrough_tests.rs @@ -45,14 +45,23 @@ const VML: &str = r##" Vec { let workbook = format!( r#""# ); let sheet1 = format!( r#"[1]Sheet1!$A$17SUM([1]Sheet1!A1:A3)*21410103050"# ); - let sheet2 = - format!(r#"1"#); + let sheet2 = if twin { + sheet1.clone() + } else { + format!(r#"1"#) + }; let root_rels = rels(&[("rId1", "officeDocument", "xl/workbook.xml"), ("rId2", "custom-properties", "docProps/custom.xml")]); let wb_rels = rels(&[ ("rId1", "worksheet", "worksheets/sheet1.xml"), @@ -74,8 +83,8 @@ fn rich_package() -> Vec { let link_rels = rels(&[("rId1", "externalLinkPath", "file:///C:/never/opened/Book2.xlsx")]); let vml_rels = rels(&[("rId1", "image", "../media/image1.emf")]); let item_props = r#""#; - make_zip(&[ - ("[Content_Types].xml", CONTENT_TYPES.as_bytes()), + let mut files: Vec<(&str, &[u8])> = vec![ + ("[Content_Types].xml", content_types.as_bytes()), ("_rels/.rels", root_rels.as_bytes()), ("docProps/custom.xml", CUSTOM_PROPS.as_bytes()), ("xl/workbook.xml", workbook.as_bytes()), @@ -97,7 +106,11 @@ fn rich_package() -> Vec { ("xl/externalLinks/externalLink1.xml", EXTERNAL_LINK.as_bytes()), ("xl/externalLinks/_rels/externalLink1.xml.rels", link_rels.as_bytes()), ("xl/connections.xml", CONNECTIONS.as_bytes()), - ]) + ]; + if twin { + files.push(("xl/worksheets/_rels/sheet2.xml.rels", sheet1_rels.as_bytes())); + } + make_zip(&files) } /// An unpacked package: parts by name, content types, resolved relationships. @@ -428,3 +441,45 @@ fn oversized_kept_parts_are_dropped() { assert!(wb.preserved.is_none()); assert!(rep.warnings.iter().any(|w| w.contains("customXml") && w.contains("dropped")), "{:?}", rep.warnings); } + +#[test] +fn kept_content_types_are_escaped_on_save() { + // A content type holding `"` and `<` (legal once unescaped by the XML reader) must not break + // out of the attribute in the saved [Content_Types].xml. + let hostile = r#"application/x-test" PartName="/x"/><Default Extension="evil"#; + let ct = CONTENT_TYPES.replace("application/vnd.openxmlformats-officedocument.customXmlProperties+xml", hostile); + let (wb, rep) = read_xlsx(&rich_package_with(&ct, false)).unwrap(); + assert!(wb.preserved.is_some(), "{:?}", rep.warnings); + let u = Unpacked::new(&write_xlsx(&wb).unwrap()); + let types = xml::parse(u.text("[Content_Types].xml").as_bytes()).unwrap(); + assert!(!types.kids("Default").any(|d| d.attr("Extension") == Some("evil"))); + let item = types.kids("Override").find(|o| o.attr("PartName") == Some("/customXml/itemProps1.xml")).unwrap(); + assert_eq!(item.attr("ContentType"), Some(r#"application/x-test" PartName="/x"/>() + }; + // Two sheets share one VML drawing: each would keep its own copy. With room for one sheet's + // content only, the second sheet's VML (and the controls drawn from it) is dropped. + crate::passthrough::KEPT_TOTAL_FOR_TEST.with(|c| c.set(one + VML.len() / 2)); + let read = read_xlsx(&rich_package_with(CONTENT_TYPES, true)); + crate::passthrough::KEPT_TOTAL_FOR_TEST.with(|c| c.set(crate::passthrough::MAX_KEPT_TOTAL)); + let (wb, rep) = read.unwrap(); + let kept = wb.preserved.as_ref().map_or(0, |p| p.byte_len()) + + wb.sheets.iter().filter_map(|s| s.preserved.as_ref()).map(|p| p.byte_len()).sum::(); + assert!(kept <= one + VML.len() / 2, "kept {kept} bytes"); + assert!(wb.sheet(0).unwrap().preserved.as_ref().is_some_and(|p| p.vml.is_some())); + assert!(!wb.sheet(1).unwrap().preserved.as_ref().is_some_and(|p| p.vml.is_some())); + assert!(rep.warnings.iter().any(|w| w.contains("\"Other\"") && w.contains("too large")), "{:?}", rep.warnings); + // Both sheets keep their drawings when there is room. + let (wb, _) = read_xlsx(&rich_package_with(CONTENT_TYPES, true)).unwrap(); + assert!(wb.sheets.iter().all(|s| s.preserved.as_ref().is_some_and(|p| p.vml.is_some()))); +} diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index 95fbed9..dc2ddc4 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -375,10 +375,11 @@ pub fn write_xlsx(wb: &Workbook) -> Result, IoError> { // Content types first, then the parts in creation order. let mut ct = format!("{XML_DECL}"); for (ext, t) in &out.defaults { - let _ = write!(ct, ""); + // Kept parts bring their content types (and extensions) from the input file: escape them. + let _ = write!(ct, "", esc_attr(ext), esc_attr(t)); } for (part, t) in &out.overrides { - let _ = write!(ct, "", esc_attr(part)); + let _ = write!(ct, "", esc_attr(part), esc_attr(t)); } ct.push_str(""); let mut zip = ZipOut::new();