From 11c8c141d62a0924e9fed41f264457baf30a80d2 Mon Sep 17 00:00:00 2001 From: InkJazz <224395883+Fanfulla@users.noreply.github.com> Date: Mon, 31 Aug 2026 16:21:32 +0200 Subject: [PATCH] fix(xlsx): retain embedded worksheet images --- src/formats/docx/content.rs | 20 +++++--- src/formats/pptx/mod.rs | 20 +++++--- src/formats/sheet/xlsx.rs | 98 ++++++++++++++++++++++++++++++++++-- src/package/relationships.rs | 5 +- src/shared/assets.rs | 7 ++- 5 files changed, 122 insertions(+), 28 deletions(-) diff --git a/src/formats/docx/content.rs b/src/formats/docx/content.rs index c5a7c18b..71b35089 100644 --- a/src/formats/docx/content.rs +++ b/src/formats/docx/content.rs @@ -66,7 +66,7 @@ impl<'a, 'b> Ctx<'a, 'b> { /// part. Failures degrade (log + `None`) per the unified policy; /// resource-limit errors always propagate. fn rel_part(&self, rel_id: &str) -> Result, ConvertError> { - rel_target_bytes(self.pkg, &self.rels, &self.base_part, rel_id) + rel_target_bytes(&mut self.pkg.borrow_mut(), &self.rels, &self.base_part, rel_id) } fn add_asset( @@ -618,13 +618,17 @@ impl<'a, 'b, 'e> InlineWalker<'a, 'b, 'e> { if let Some(rel_id) = image_rel { // External-mode targets (`r:link`) become external image // sources; embedded targets are retained as assets. - let source = crate::shared::assets::rel_image_source( - self.ctx.pkg, - &self.ctx.rels, - &self.ctx.base_part, - self.ctx.assets, - rel_id, - )?; + let source = { + let mut pkg = self.ctx.pkg.borrow_mut(); + let mut assets = self.ctx.assets.borrow_mut(); + crate::shared::assets::rel_image_source( + &mut pkg, + &self.ctx.rels, + &self.ctx.base_part, + &mut assets, + rel_id, + )? + }; match source { Some(source) => self.push(Inline::Image { alt: descr, source }), None => { diff --git a/src/formats/pptx/mod.rs b/src/formats/pptx/mod.rs index ce6c6647..41ceeb74 100644 --- a/src/formats/pptx/mod.rs +++ b/src/formats/pptx/mod.rs @@ -316,7 +316,7 @@ impl SlideCtx<'_, '_> { /// part. Failures degrade (log + `None`) per the unified policy; /// resource-limit errors always propagate. fn rel_part(&self, rel_id: &str) -> Result, ConvertError> { - rel_target_bytes(self.pkg, self.rels, self.base_part, rel_id) + rel_target_bytes(&mut self.pkg.borrow_mut(), self.rels, self.base_part, rel_id) } } @@ -361,13 +361,17 @@ fn parse_shapes( // External-mode targets (`r:link`) become external image // sources; embedded targets are retained as assets. let source = match rid { - Some(rid) => crate::shared::assets::rel_image_source( - ctx.pkg, - ctx.rels, - ctx.base_part, - ctx.assets, - rid, - )?, + Some(rid) => { + let mut pkg = ctx.pkg.borrow_mut(); + let mut assets = ctx.assets.borrow_mut(); + crate::shared::assets::rel_image_source( + &mut pkg, + ctx.rels, + ctx.base_part, + &mut assets, + rid, + )? + } None => None, }; if source.is_some() || !descr.trim().is_empty() { diff --git a/src/formats/sheet/xlsx.rs b/src/formats/sheet/xlsx.rs index e0924562..33ecd51e 100644 --- a/src/formats/sheet/xlsx.rs +++ b/src/formats/sheet/xlsx.rs @@ -1,7 +1,8 @@ //! In-house SpreadsheetML reader (.xlsx / .xlsm): the workbook's visible -//! sheets, shared strings, cell number formats from `xl/styles.xml`, and -//! merge regions. Rows, columns, and sheets the source hides are omitted, -//! and merge regions are remapped onto the surviving grid. +//! sheets, shared strings, cell number formats from `xl/styles.xml`, merge +//! regions, and embedded worksheet assets. Rows, columns, and sheets the +//! source hides are omitted, and merge regions are remapped onto the surviving +//! grid. use super::controls::{Checkboxes, cell_inlines, read_vml_checkboxes}; use super::numfmt::{DateParts, NumberFormat, Rendered, builtin_code}; @@ -9,9 +10,12 @@ use super::{format_duration_days, format_float, format_time_of_day}; use crate::error::ConvertError; use crate::model::{Block, Cell, Document, GridBuilder, Inline, Table, TableKind}; use crate::package::limits; -use crate::package::relationships::{Relationships, read_rels, rel_type, rels_part_for}; +use crate::package::relationships::{ + Relationships, TargetMode, read_rels, rel_target_bytes, rel_type, rels_part_for, +}; use crate::package::xml::{Element, ns}; use crate::package::{Package, path}; +use crate::shared::assets::{AssetSink, rel_image_source}; use crate::shared::header::resolve_header_rows; use crate::shared::text::clean_text; use std::collections::{HashMap, HashSet}; @@ -19,6 +23,9 @@ use std::rc::Rc; pub(super) const SHARED_STRINGS_REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/sharedStrings"; +const DRAWING_REL: &str = + "http://schemas.openxmlformats.org/officeDocument/2006/relationships/drawing"; +const IMAGE_REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/image"; /// The grid bounds the format defines; a reference outside them is not a /// real cell. @@ -72,6 +79,7 @@ pub(super) fn parse(pkg: &mut Package, wb_part: &str) -> Result 1; let mut doc = Document::default(); + let mut assets = AssetSink::new(); let mut failed = 0usize; // One budget for the workbook, so sheets cannot multiply the cap. let mut slots = 0u64; @@ -82,6 +90,7 @@ pub(super) fn parse(pkg: &mut Package, wb_part: &str) -> Result Result drawing -> image. +fn collect_sheet_assets( + pkg: &mut Package, + worksheet: &Element, + sheet_part: &str, + assets: &mut AssetSink, +) -> Result<(), ConvertError> { + let sheet_rels = read_rels(pkg, &rels_part_for(sheet_part))?; + for drawing in worksheet.find_all(ns::SML, "drawing") { + let Some(drawing_rel_id) = drawing.attr_qualified(ns::R, "id") else { + continue; + }; + let Some(drawing_rel) = sheet_rels.get(drawing_rel_id) else { + continue; + }; + if drawing_rel.mode != TargetMode::Internal || drawing_rel.rel_type != DRAWING_REL { + continue; + } + let Some((drawing_part, drawing_bytes)) = + rel_target_bytes(pkg, &sheet_rels, sheet_part, drawing_rel_id)? + else { + continue; + }; + let drawing = match crate::package::xml::parse_xml(&drawing_bytes) { + Ok(drawing) => drawing, + Err(e) if e.is_fatal() => return Err(e), + Err(e) => { + log::warn!("skipping corrupt drawing part {drawing_part}: {e}"); + continue; + } + }; + let drawing_rels = read_rels(pkg, &rels_part_for(&drawing_part))?; + for blip in drawing.descendants(ns::A, "blip") { + let Some(image_rel_id) = blip.attr_qualified(ns::R, "embed") else { + continue; + }; + let Some(image_rel) = drawing_rels.get(image_rel_id) else { + continue; + }; + if image_rel.mode != TargetMode::Internal || image_rel.rel_type != IMAGE_REL { + continue; + } + let _ = rel_image_source(pkg, &drawing_rels, &drawing_part, assets, image_rel_id)?; + } + } + Ok(()) +} + /// Part name for a workbook-level sibling: the relationship of the given /// type when present, else the conventional name next to the workbook part. pub(super) fn sibling_part_name( @@ -699,7 +759,6 @@ mod tests { "http://schemas.openxmlformats.org/officeDocument/2006/relationships/worksheet"; const STYLES_REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/styles"; - /// Route through the shared container dispatch, the only entry a caller /// has. fn parse(bytes: &[u8]) -> Result { @@ -883,6 +942,35 @@ mod tests { assert_eq!(texts(first_table(&doc)), vec![vec!["plain", "rich"]]); } + #[test] + fn embedded_xlsx_images_are_retained_as_assets() { + let sheet_rels = format!( + r#""# + ); + let drawing_rels = format!( + r#""# + ); + let sheet = r#"visual evidence"#; + let drawing = r#""#; + let wb = Wb { + sheets: vec![("S", "", sheet)], + extra: vec![ + ("xl/worksheets/_rels/sheet1.xml.rels", &sheet_rels), + ("xl/drawings/drawing1.xml", drawing), + ("xl/drawings/_rels/drawing1.xml.rels", &drawing_rels), + ("xl/media/image1.png", "embedded-image"), + ], + ..Wb::default() + }; + + let doc = parse(&wb.build()).unwrap(); + + assert_eq!(doc.assets.len(), 1); + assert_eq!(doc.assets[0].media_type, "image/png"); + assert_eq!(doc.assets[0].origin_part, "xl/media/image1.png"); + assert_eq!(doc.assets[0].bytes, b"embedded-image"); + } + #[test] fn date1904_serials_shift_epoch() { let wb = Wb { diff --git a/src/package/relationships.rs b/src/package/relationships.rs index ecb74ff8..5e115845 100644 --- a/src/package/relationships.rs +++ b/src/package/relationships.rs @@ -3,7 +3,6 @@ use crate::error::ConvertError; use crate::package::archive::Package; use crate::package::xml::{normalize_ooxml_uri, ns}; -use std::cell::RefCell; use std::collections::HashMap; use std::rc::Rc; @@ -95,7 +94,7 @@ pub type RelTarget = (String, Rc<[u8]>); /// unresolvable or unreadable targets degrade (log + `None`) per the unified /// policy; fatal resource-limit errors always propagate. pub fn rel_target_bytes( - pkg: &RefCell, + pkg: &mut Package, rels: &Relationships, base_part: &str, rel_id: &str, @@ -113,7 +112,7 @@ pub fn rel_target_bytes( return Ok(None); } }; - match pkg.borrow_mut().optional_part(&target.path)? { + match pkg.optional_part(&target.path)? { Some(bytes) => Ok(Some((target.path, bytes))), None => { log::warn!("relationship target {} is missing", target.path); diff --git a/src/shared/assets.rs b/src/shared/assets.rs index 9cce28db..ab14dec1 100644 --- a/src/shared/assets.rs +++ b/src/shared/assets.rs @@ -5,7 +5,6 @@ use crate::model::{Asset, AssetId, ImageSource}; use crate::package::Package; use crate::package::limits; use crate::package::relationships::{Relationships, TargetMode, rel_target_bytes}; -use std::cell::RefCell; #[derive(Default)] pub struct AssetSink { @@ -52,10 +51,10 @@ impl AssetSink { /// package and are retained as assets. Failures degrade to `None` per the /// unified policy; fatal errors propagate. pub fn rel_image_source( - pkg: &RefCell, + pkg: &mut Package, rels: &Relationships, base_part: &str, - assets: &RefCell, + assets: &mut AssetSink, rel_id: &str, ) -> Result, ConvertError> { let Some(rel) = rels.get(rel_id) else { @@ -67,7 +66,7 @@ pub fn rel_image_source( match rel_target_bytes(pkg, rels, base_part, rel_id)? { Some((part, bytes)) => { let media = media_type_for(&part); - let id = assets.borrow_mut().add(media, part, &bytes)?; + let id = assets.add(media, part, &bytes)?; Ok(Some(ImageSource::Asset(id))) } None => Ok(None),