Merge PR #1248: Opening an XLSX with a part over 512 MB says how to get the data in

crates/engine/src/io.rs

@@ -147,7 +147,12 @@ fn open_plain(name: &str, bytes: &[u8], csv_as_text: bool) -> Result<(Workbook,
     }
     // Text named `.xlsx` is read as text.
     if sniffed == gridcraft_xlsx::Format::Xlsx || (kind == Some(FileKind::Xlsx) && sniffed != gridcraft_xlsx::Format::Csv) {
-        let (wb, report) = gridcraft_xlsx::read_xlsx(bytes).map_err(|e| EngineError::Other(format!("We can't open '{name}': {e}")))?;
+        let (wb, report) = gridcraft_xlsx::read_xlsx(bytes).map_err(|e| {
+            EngineError::Other(match (&e, gridcraft_xlsx::oversized_part(bytes)) {
+                (gridcraft_xlsx::IoError::TooLarge(_), Some((part, size))) => part_too_large_message(name, &part, size),
+                _ => format!("We can't open '{name}': {e}"),
+            })
+        })?;
         return Ok((wb, report.warnings));
     }
     match kind {
@@ -176,6 +181,25 @@ fn open_plain(name: &str, bytes: &[u8], csv_as_text: bool) -> Result<(Workbook,
     }
 }
 
+/// What to tell the user when one part of an XLSX package unpacks to more than the reader's
+/// per-part limit: the sizes, why the limit is there, and what gets the data in. The limit is
+/// per part, so smaller sheets fit; the shared strings are one part for every sheet, so only a
+/// CSV, which has no parts, helps there.
+fn part_too_large_message(name: &str, part: &str, size: u64) -> String {
+    let limit = gridcraft_xlsx::MAX_PART >> 20;
+    let mb = size >> 20;
+    let size = if mb > limit { format!("{mb} MB") } else { format!("more than {limit} MB") };
+    let remedy = if part.contains("worksheets/") {
+        "To open it, split the data across several sheets, since the limit applies to each part, or save the sheet as CSV, which keeps only its values."
+    } else {
+        "To open it, save the data as CSV, which keeps only its values."
+    };
+    format!(
+        "We can't open '{name}': its part {part} unpacks to {size}, over GridCraft's limit of {limit} MB for one part \
+         (a guard against damaged or malicious files). {remedy}"
+    )
+}
+
 /// A CSV opens as a sheet named after the file (like Excel).
 fn rename_first_sheet(wb: &mut Workbook, path: &str) {
     let stem = std::path::Path::new(path).file_stem().and_then(|s| s.to_str()).unwrap_or("Sheet1");
@@ -881,6 +905,54 @@ mod tests {
         );
     }
 
+    /// Where `part`'s entry starts in the central directory of `package`.
+    fn central_entry(package: &[u8], part: &str) -> usize {
+        let mut at = 0;
+        while let Some(i) = package.get(at..).and_then(|rest| rest.windows(4).position(|w| w == b"PK\x01\x02")) {
+            let entry = at + i;
+            let len = u16::from_le_bytes([package[entry + 28], package[entry + 29]]) as usize;
+            if &package[entry + 46..entry + 46 + len] == part.as_bytes() {
+                return entry;
+            }
+            at = entry + 46;
+        }
+        panic!("{part} is not in the package");
+    }
+
+    /// `package` with `part` declaring `size` unpacked bytes: the reader refuses the part on the
+    /// declared size, so no real 512 MB part has to be built.
+    fn declaring(mut package: Vec<u8>, part: &str, size: u32) -> Vec<u8> {
+        let at = central_entry(&package, part);
+        package[at + 24..at + 28].copy_from_slice(&size.to_le_bytes());
+        package
+    }
+
+    #[test]
+    fn a_part_over_the_size_limit_says_how_to_get_the_data_in() {
+        let mut wb = Workbook::new();
+        wb.sheet_mut(0).unwrap().set_value(CellRef::new(0, 0), gridcraft_core::Value::text("big"));
+        let package = save_bytes(&wb, "big.xlsx").unwrap();
+        // The size reported in storytold/gridcraft#1242.
+        let msg = open_bytes("big.xlsx", &declaring(package.clone(), "xl/worksheets/sheet1.xml", 540_278_731)).unwrap_err().to_string();
+        assert!(msg.starts_with("We can't open 'big.xlsx': "), "{msg}");
+        assert!(msg.contains("xl/worksheets/sheet1.xml") && msg.contains("515 MB") && msg.contains("512 MB"), "{msg}");
+        assert!(msg.contains("several sheets") && msg.contains("CSV"), "{msg}");
+        assert!(!msg.contains("bytes"), "{msg}");
+        // Splitting sheets doesn't shrink the shared strings, which every sheet uses: only CSV helps.
+        let msg = open_bytes("big.xlsx", &declaring(package.clone(), "xl/sharedStrings.xml", 540_278_731)).unwrap_err().to_string();
+        assert!(msg.contains("xl/sharedStrings.xml") && msg.contains("515 MB") && msg.contains("CSV"), "{msg}");
+        assert!(!msg.contains("several sheets"), "{msg}");
+        // Just over the limit never reads as a size equal to it.
+        let msg = open_bytes("big.xlsx", &declaring(package, "xl/worksheets/sheet1.xml", 512 * 1024 * 1024 + 1)).unwrap_err().to_string();
+        assert!(msg.contains("more than 512 MB"), "{msg}");
+        // A package that fails for another reason keeps its own error, oversized part or not.
+        let mut broken = declaring(save_bytes(&wb, "big.xlsx").unwrap(), "xl/worksheets/sheet1.xml", 540_278_731);
+        let at = central_entry(&broken, "xl/workbook.xml");
+        broken[at + 46 + "xl/workbook.xml".len() - 1] = b'x';
+        let msg = open_bytes("big.xlsx", &broken).unwrap_err().to_string();
+        assert!(msg.starts_with("We can't open 'big.xlsx': ") && !msg.contains("unpacks"), "{msg}");
+    }
+
     #[test]
     fn content_wins_over_a_mismatched_extension() {
         let mut wb = Workbook::new();

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 package::MAX_PART;
 pub use passthrough::rebase_links;
 pub use xls::read_xls;
 pub use xlsb::read_xlsb;
@@ -85,6 +86,12 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> {
     read::read_xlsx(bytes)
 }
 
+/// The part of a zip package that declares more than [`MAX_PART`] unpacked bytes, with that size:
+/// what an [`IoError::TooLarge`] from reading the package is about when one part is too large.
+pub fn oversized_part(bytes: &[u8]) -> Option<(String, u64)> {
+    package::oversized_part(bytes)
+}
+
 /// Which member of the SpreadsheetML family a package is saved as. Only the content type of the
 /// workbook part differs (ECMA-376 Part 1 Workbook part; MS-OI29500 adds the macro-enabled types).
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]

crates/xlsx/src/package.rs

@@ -120,6 +120,15 @@ impl<'a> Package<'a> {
     }
 }
 
+/// The first part whose declared unpacked size is over [`MAX_PART`], with that size.
+pub fn oversized_part(bytes: &[u8]) -> Option<(String, u64)> {
+    let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).ok()?;
+    (0..zip.len().min(MAX_ENTRIES)).find_map(|i| {
+        let f = zip.by_index_raw(i).ok()?;
+        (f.size() > MAX_PART).then(|| (f.name().to_string(), f.size()))
+    })
+}
+
 fn prefix(part: &str, e: IoError) -> IoError {
     match e {
         IoError::Xml(m) => IoError::Xml(format!("{part}: {m}")),