Merge pull request #824 from marcoarnulfo:fix/xlsx-rebase-external-link-paths

apps/gridcraft-cli/src/main.rs

@@ -277,7 +277,7 @@ fn convert(args: &[String]) -> Result<(), String> {
     let mut s = open(input)?;
     activate_sheet(&mut s, a.opt("sheet"))?;
     let d = s.doc().map_err(|e| e.to_string())?;
-    let bytes = gridcraft_engine::io::save_bytes(&d.wb, out).map_err(|e| e.to_string())?;
+    let bytes = gridcraft_engine::io::save_file_bytes(&d.wb, out).map_err(|e| e.to_string())?;
     gridcraft_engine::io::write_file(out, &bytes).map_err(|e| e.to_string())?;
     eprintln!("wrote {out} ({} bytes)", bytes.len());
     Ok(())

crates/engine/src/cmd/file.rs

@@ -56,7 +56,8 @@ fn open(s: &mut Session, p: &Json) -> Result<Json> {
         return ok();
     };
     let encrypted = gridcraft_xlsx::sniff(&bytes) == gridcraft_xlsx::Format::Encrypted;
-    let (wb, warnings) = crate::io::open_bytes_with_password(&name, &bytes, str_param(p, "password"))?;
+    let (mut wb, warnings) = crate::io::open_bytes_with_password(&name, &bytes, str_param(p, "password"))?;
+    wb.source_dir = str_param(p, "path").and_then(crate::io::source_dir);
     let imported =
         matches!(crate::io::FileKind::from_path(&name), Some(crate::io::FileKind::Ods | crate::io::FileKind::Xlsb | crate::io::FileKind::Xls))
             || matches!(gridcraft_xlsx::sniff(&bytes), gridcraft_xlsx::Format::Ods | gridcraft_xlsx::Format::Xlsb | gridcraft_xlsx::Format::Xls);
@@ -110,11 +111,13 @@ fn save(s: &mut Session, p: &Json) -> Result<Json> {
     }
     let d = s.doc_mut()?;
     // The document's own file keeps the format its content had when opened; any other path
-    // goes by its extension.
+    // goes by its extension. Relative paths to linked workbooks are rewritten for the new folder.
     let kind = crate::io::FileKind::from_path(&path).or(d.content_kind.filter(|_| d.path.as_deref() == Some(path.as_str())));
+    let rebased = crate::io::source_dir(&path).and_then(|f| gridcraft_xlsx::rebase_links(&d.wb, &f));
+    let wb = rebased.as_ref().unwrap_or(&d.wb);
     let bytes = match kind {
-        Some(kind) => crate::io::save_bytes_as(&d.wb, &path, kind)?,
+        Some(kind) => crate::io::save_bytes_as(wb, &path, kind)?,
-        None => crate::io::save_bytes(&d.wb, &path)?,
+        None => crate::io::save_bytes(wb, &path)?,
     };
     crate::io::write_file(&path, &bytes)?;
     // Only a full-fidelity format clears the dirty flag and becomes the document's file.

crates/engine/src/io.rs

@@ -187,6 +187,20 @@ pub fn save_bytes_as(wb: &Workbook, path: &str, kind: FileKind) -> Result<Vec<u8
     }
 }
 
+/// [`save_bytes`] for the file at `path`: relative paths to linked workbooks are rewritten for
+/// its folder, so they still name the same files.
+pub fn save_file_bytes(wb: &Workbook, path: &str) -> Result<Vec<u8>> {
+    match source_dir(path).and_then(|f| gridcraft_xlsx::rebase_links(wb, &f)) {
+        Some(rebased) => save_bytes(&rebased, path),
+        None => save_bytes(wb, path),
+    }
+}
+
+/// The folder of the file at `path`, which relative paths to linked workbooks are relative to.
+pub fn source_dir(path: &str) -> Option<std::path::PathBuf> {
+    std::path::absolute(path).ok()?.parent().map(std::path::Path::to_path_buf)
+}
+
 /// Save As CSV/Text writes "the text and values as they are displayed in cells" with "all
 /// characters in each cell" (Microsoft Support, *Excel formatting and features that are not
 /// transferred to other file formats*): number formats applied, never column-width `#####`.

crates/engine/src/tests.rs

@@ -1620,6 +1620,80 @@ fn saving_an_opened_semicolon_csv_keeps_semicolons() {
     std::fs::remove_dir_all(dir).unwrap();
 }
 
+/// A workbook whose B1 is `=[1]Sheet1!A1`, linked to `link` (an external relationship target).
+fn linked_workbook(link: &str) -> Vec<u8> {
+    use std::io::{Cursor, Write};
+    const NS_REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships";
+    let types = r#"<Types xmlns="http://schemas.openxmlformats.org/package/2006/content-types"><Override PartName="/xl/workbook.xml" ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet.main+xml"/><Override PartName="/xl/worksheets/sheet1.xml" ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.worksheet+xml"/><Override PartName="/xl/externalLinks/externalLink1.xml" ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.externalLink+xml"/></Types>"#.to_string();
+    let rels = format!(
+        r#"<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="{NS_REL}/officeDocument" Target="xl/workbook.xml"/></Relationships>"#
+    );
+    let book = format!(
+        r#"<workbook xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" xmlns:r="{NS_REL}"><sheets><sheet name="Sheet1" sheetId="1" r:id="rId1"/></sheets><externalReferences><externalReference r:id="rId2"/></externalReferences></workbook>"#
+    );
+    let book_rels = format!(
+        r#"<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="{NS_REL}/worksheet" Target="worksheets/sheet1.xml"/><Relationship Id="rId2" Type="{NS_REL}/externalLink" Target="externalLinks/externalLink1.xml"/></Relationships>"#
+    );
+    let sheet = r#"<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><sheetData><row r="1"><c r="B1"><f>[1]Sheet1!A1</f><v>10</v></c></row></sheetData></worksheet>"#.to_string();
+    let link_part = format!(
+        r#"<externalLink xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" xmlns:r="{NS_REL}"><externalBook r:id="rId1"><sheetNames><sheetName val="Sheet1"/></sheetNames><sheetDataSet><sheetData sheetId="0"><row r="1"><cell r="A1"><v>10</v></cell></row></sheetData></sheetDataSet></externalBook></externalLink>"#
+    );
+    let link_rels = format!(
+        r#"<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="{NS_REL}/externalLinkPath" Target="{link}" TargetMode="External"/></Relationships>"#
+    );
+    let mut zip = zip::ZipWriter::new(Cursor::new(Vec::new()));
+    for (name, data) in [
+        ("[Content_Types].xml", types),
+        ("_rels/.rels", rels),
+        ("xl/workbook.xml", book),
+        ("xl/_rels/workbook.xml.rels", book_rels),
+        ("xl/worksheets/sheet1.xml", sheet),
+        ("xl/externalLinks/externalLink1.xml", link_part),
+        ("xl/externalLinks/_rels/externalLink1.xml.rels", link_rels),
+    ] {
+        zip.start_file(name, zip::write::SimpleFileOptions::default()).unwrap();
+        zip.write_all(data.as_bytes()).unwrap();
+    }
+    zip.finish().unwrap().into_inner()
+}
+
+/// The target of the linked workbook in a saved file.
+fn link_target(path: &std::path::Path) -> String {
+    use std::io::Read;
+    let mut zip = zip::ZipArchive::new(std::io::Cursor::new(std::fs::read(path).unwrap())).unwrap();
+    let mut rels = String::new();
+    zip.by_name("xl/externalLinks/_rels/externalLink1.xml.rels").unwrap().read_to_string(&mut rels).unwrap();
+    let (_, after) = rels.split_once("Target=\"").unwrap();
+    after.split_once('"').unwrap().0.to_string()
+}
+
+/// Save As into another folder keeps the link on the same file, as Excel does: the relative path
+/// is rewritten from the new folder (#801).
+#[test]
+#[cfg(not(target_arch = "wasm32"))]
+fn save_as_into_another_folder_keeps_linked_workbooks_on_the_same_file() {
+    #[allow(clippy::disallowed_methods)] // native-only test: a unique temp dir name
+    let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos();
+    let a = std::env::temp_dir().join(format!("gridcraft-links-{}-{nonce}", std::process::id()));
+    let b = a.join("b");
+    std::fs::create_dir_all(&b).unwrap();
+    let dest = a.join("Dest.xlsx");
+    std::fs::write(&dest, linked_workbook("Source.xlsx")).unwrap();
+    let mut s = s();
+    s.execute("file.open", json!({"path": dest})).unwrap();
+    s.execute("file.saveAs", json!({"path": b.join("Dest.xlsx")})).unwrap();
+    assert_eq!(link_target(&b.join("Dest.xlsx")), "../Source.xlsx");
+    // Saved again into the folder it was read from, the path is the one it was read with.
+    s.execute("file.saveAs", json!({"path": a.join("Copy.xlsx")})).unwrap();
+    assert_eq!(link_target(&a.join("Copy.xlsx")), "Source.xlsx");
+    // Read from a subfolder and saved into its parent, the path goes down into the subfolder.
+    let deep = b.join("Deep.xlsx");
+    std::fs::write(&deep, linked_workbook("Source.xlsx")).unwrap();
+    let mut s = Session::new();
+    s.execute("file.open", json!({"path": deep})).unwrap();
+    s.execute("file.saveAs", json!({"path": a.join("Deep.xlsx")})).unwrap();
+    assert_eq!(link_target(&a.join("Deep.xlsx")), "b/Source.xlsx");
+    std::fs::remove_dir_all(a).unwrap();
 /// A typo like `report.cvs` must not quietly write an XLSX under that name and adopt it as the
 /// document's file.
 #[test]

crates/model/src/workbook.rs

@@ -116,6 +116,10 @@ pub struct Workbook {
     /// Unmodelled file content kept for saving (package root and workbook level). Not in JSON.
     #[serde(skip)]
     pub preserved: Option<Arc<crate::preserved::Preserved>>,
+    /// The folder the workbook was read from, which relative paths to linked workbooks in
+    /// `preserved` are relative to. Saving into another folder rewrites them from there. Not in JSON.
+    #[serde(skip)]
+    pub source_dir: Option<std::path::PathBuf>,
 }
 
 impl Default for Workbook {
@@ -143,6 +147,7 @@ impl Workbook {
             vba_project: None,
             csv_delimiter: None,
             preserved: None,
+            source_dir: None,
         }
     }
     pub fn sheet(&self, i: usize) -> Option<&Sheet> {

crates/xlsx/src/lib.rs

@@ -47,6 +47,7 @@ use gridcraft_model::Workbook;
 pub use crypto::{DecryptError, check_encryption, decrypt};
 pub use csv::{MAX_CSV_BYTES, read_csv, write_csv, write_csv_limited};
 pub use ods::read_ods;
+pub use passthrough::rebase_links;
 pub use xls::read_xls;
 pub use xlsb::read_xlsb;
 

crates/xlsx/src/passthrough.rs

@@ -15,7 +15,8 @@
 //!   drawings, charts, tables, PivotTables, comments, slicers…) is not kept: it would point at
 //!   content that no longer exists in the same form.
 //! - External relationship targets (the path of a linked workbook) are kept as text and never
-//!   opened, followed or refreshed.
+//!   opened, followed or refreshed. A relative path to a linked workbook is rewritten when the
+//!   workbook is saved into another folder ([`rebase_links`]), so it still names the same file.
 //! - Sheet content lives on the sheet: deleting the sheet drops it, Copy Sheet doesn't copy it.
 //!   Cell references inside kept parts (a control's linked cell, a query table's range) are kept
 //!   as they were, not adjusted to inserted or deleted rows.
@@ -25,6 +26,7 @@
 use std::borrow::Cow;
 use std::collections::{HashMap, HashSet, VecDeque};
 use std::fmt::Write as _;
+use std::path::{Component, Path};
 use std::sync::Arc;
 
 use gridcraft_model::{Preserved, PreservedPart, PreservedRel, PreservedSheet, PreservedXml, Sheet, Visibility, Workbook};
@@ -1160,3 +1162,136 @@ impl<'a> SheetKept<'a> {
         s
     }
 }
+
+// ---------------------------------------------------------------- linked workbooks
+
+/// `wb` as it is saved into the folder `to` (absolute): the relative paths of its linked
+/// workbooks, relative to the folder it was read from, are rewritten relative to `to`, as Excel
+/// does on Save As, or made absolute when no relative path exists (another drive). `None` when
+/// nothing changes.
+pub fn rebase_links(wb: &Workbook, to: &Path) -> Option<Workbook> {
+    let p = wb.preserved.as_deref()?;
+    let from = Folder::of(wb.source_dir.as_deref()?)?;
+    let to = Folder::of(to)?;
+    if from == to {
+        return None;
+    }
+    let mut parts = p.parts.clone();
+    let mut changed = false;
+    for r in parts.iter_mut().flat_map(|part| part.rels.iter_mut()) {
+        if r.external
+            && is_kind(r, &["externalLinkPath"])
+            && let Some(target) = rebase(&r.target, &from, &to)
+        {
+            r.target = target;
+            changed = true;
+        }
+    }
+    if !changed {
+        return None;
+    }
+    let mut out = wb.clone();
+    out.preserved = Some(Arc::new(Preserved { parts, ..p.clone() }));
+    Some(out)
+}
+
+/// An absolute folder, lexically normalized.
+#[derive(PartialEq)]
+struct Folder {
+    /// Drive or share and root separator (`/`, `C:\`, `\\server\share\`).
+    root: String,
+    names: Vec<String>,
+}
+
+impl Folder {
+    fn of(dir: &Path) -> Option<Folder> {
+        if !dir.is_absolute() {
+            return None;
+        }
+        let mut f = Folder { root: String::new(), names: vec![] };
+        for c in dir.components() {
+            match c {
+                Component::Prefix(_) | Component::RootDir => f.root.push_str(&c.as_os_str().to_string_lossy()),
+                Component::CurDir => {}
+                Component::ParentDir => {
+                    f.names.pop();
+                }
+                Component::Normal(n) => f.names.push(n.to_string_lossy().into_owned()),
+            }
+        }
+        Some(f)
+    }
+}
+
+/// A relative link target read from `from`, rewritten for `to`. `None` keeps it as it is: an
+/// absolute path or URL, or a path that climbs above the root.
+fn rebase(target: &str, from: &Folder, to: &Folder) -> Option<String> {
+    if !is_relative_path(target) {
+        return None;
+    }
+    // Each name as the file system knows it, and as it is written in a target. Names taken from
+    // the original target keep their spelling.
+    let mut path: Vec<(String, String)> = from.names.iter().map(|n| (n.clone(), encode_name(n))).collect();
+    for seg in target.split(['/', '\\']) {
+        match seg {
+            "" | "." => {}
+            ".." => {
+                path.pop()?;
+            }
+            _ => path.push((decode_name(seg), seg.to_string())),
+        }
+    }
+    let folders = path.len().checked_sub(1)?;
+    if !from.root.eq_ignore_ascii_case(&to.root) {
+        let sep = if from.root.contains('\\') { "\\" } else { "/" };
+        let names: Vec<&str> = path.iter().map(|(name, _)| name.as_str()).collect();
+        return Some(format!("file:///{}{}", from.root, names.join(sep)));
+    }
+    let common = to.names.iter().zip(&path).take(folders).take_while(|(a, (b, _))| *a == b).count();
+    let up = "../".repeat(to.names.len().saturating_sub(common));
+    let down: Vec<&str> = path.iter().skip(common).map(|(_, written)| written.as_str()).collect();
+    Some(format!("{up}{}", down.join("/")))
+}
+
+/// Not empty, no root, no scheme or drive (`file:`, `http:`, `C:`).
+fn is_relative_path(target: &str) -> bool {
+    let scheme = target.split_once(':').is_some_and(|(s, _)| {
+        s.starts_with(|c: char| c.is_ascii_alphabetic()) && s.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.'))
+    });
+    !target.is_empty() && !target.starts_with(['/', '\\']) && !scheme
+}
+
+/// A folder name as a segment of a relationship target (a URI): ASCII characters a URI path
+/// can't hold are percent-encoded, other characters are written as they are.
+fn encode_name(name: &str) -> String {
+    let mut s = String::with_capacity(name.len());
+    for c in name.chars() {
+        if !c.is_ascii() || c.is_ascii_alphanumeric() || "-._~!$&'()+,;=@".contains(c) {
+            s.push(c);
+        } else {
+            let _ = write!(s, "%{:02X}", c as u32);
+        }
+    }
+    s
+}
+
+/// A target segment with its percent-escapes decoded; as written when they don't decode to UTF-8.
+fn decode_name(seg: &str) -> String {
+    let b = seg.as_bytes();
+    let mut out = Vec::with_capacity(b.len());
+    let mut i = 0;
+    while let Some(&c) = b.get(i) {
+        let hex = b.get(i + 1..i + 3).and_then(|h| std::str::from_utf8(h).ok()).and_then(|h| u8::from_str_radix(h, 16).ok());
+        match hex {
+            Some(byte) if c == b'%' => {
+                out.push(byte);
+                i += 3;
+            }
+            _ => {
+                out.push(c);
+                i += 1;
+            }
+        }
+    }
+    String::from_utf8(out).unwrap_or_else(|_| seg.to_string())
+}

crates/xlsx/src/tests/passthrough_tests.rs

@@ -484,6 +484,52 @@ fn kept_vml_drawings_are_charged_to_the_kept_total() {
     assert!(wb.sheets.iter().all(|s| s.preserved.as_ref().is_some_and(|p| p.vml.is_some())));
 }
 
+/// Where a link with `target`, in a workbook read from `from`, points once saved into `to`.
+#[cfg(unix)]
+fn rebased(target: &str, from: &str, to: &str) -> String {
+    let link = |rel_type: &str| gridcraft_model::PreservedRel {
+        id: "rId1".into(),
+        rel_type: format!("{NS_R}/{rel_type}"),
+        target: target.into(),
+        external: true,
+    };
+    let part = gridcraft_model::PreservedPart {
+        name: "xl/externalLinks/externalLink1.xml".into(),
+        content_type: String::new(),
+        data: Arc::from(&b""[..]),
+        rels: vec![link("externalLinkPath"), link("hyperlink")],
+    };
+    let mut wb = gridcraft_model::Workbook::new();
+    wb.preserved = Some(Arc::new(gridcraft_model::Preserved { parts: vec![part], ..Default::default() }));
+    wb.source_dir = Some(from.into());
+    let Some(out) = crate::rebase_links(&wb, std::path::Path::new(to)) else { return target.into() };
+    let rels = &out.preserved.as_ref().unwrap().parts[0].rels;
+    assert_eq!(rels[1].target, target, "only paths of linked workbooks change");
+    rels[0].target.clone()
+}
+
+#[test]
+#[cfg(unix)]
+fn saving_into_another_folder_rebases_relative_links_to_workbooks() {
+    assert_eq!(rebased("Source.xlsx", "/links", "/links/move2"), "../Source.xlsx");
+    assert_eq!(rebased("Source.xlsx", "/links", "/other/deeper"), "../../links/Source.xlsx");
+    assert_eq!(rebased("../Source.xlsx", "/links/move2", "/links"), "Source.xlsx");
+    assert_eq!(rebased("sub\\Source.xlsx", "/links", "/links/sub"), "Source.xlsx");
+    assert_eq!(rebased("./sub/../Source.xlsx", "/links", "/links/sub/"), "../Source.xlsx");
+    // Folder names from the file system are written as a URI path; the target's own spelling stays.
+    assert_eq!(rebased("My%20Book.xlsx", "/links/q2 data", "/links"), "q2%20data/My%20Book.xlsx");
+    assert_eq!(rebased("Résumé.xlsx", "/a/Été", "/a"), "Été/Résumé.xlsx");
+    // The same folder, absolute paths, URLs and paths above the root are kept as they are.
+    assert_eq!(rebased("Source.xlsx", "/links/", "/links/x/.."), "Source.xlsx");
+    for target in ["/links/Source.xlsx", "file:///C:/links/Source.xlsx", "C:\\links\\Source.xlsx", "https://example.com/Source.xlsx", "../../x.xlsx"]
+    {
+        assert_eq!(rebased(target, "/links", "/links/move2"), target);
+    }
+    // Without the folder it was read from, nothing to rebase.
+    let (mut wb, _) = read_xlsx(&rich_package()).unwrap();
+    assert!(crate::rebase_links(&wb, std::path::Path::new("/elsewhere")).is_none());
+    wb.source_dir = Some("relative/dir".into());
+    assert!(crate::rebase_links(&wb, std::path::Path::new("/elsewhere")).is_none());
 // ------------------------------------------------------------------ chart sheets (#727)
 
 const NS_C: &str = "http://schemas.openxmlformats.org/drawingml/2006/chart";