Export for Screens: a cancel while the archive is made returns no archive

A cancel that came after the last file's check was not seen: an export
without a folder returned its files or its archive anyway. The export now
checks once more after the last file and again after deflating the
archive, so a cancel returns the cancelled error and nothing is written
or returned.

zip::deflate takes a `stop` callback that it calls before each 1 MiB of
a file. Export for Screens passes the cancel state, so deflating stops
at the next MiB. file.package passes one that never stops, so its
behavior does not change.
This commit is contained in:
dotCooCoo
2026-10-29 10:03:27 +07:00
committed by bflatastic
parent 3b01769d46
commit 2731ffacdd
4 changed files with 73 additions and 8 deletions
+6 -2
View File
@@ -818,14 +818,18 @@ fn encode_and_write(export: ScreenExport, writer: &mut dyn FnMut(&str, &[u8]) ->
files += 1;
}
}
// A cancel that came while the last file was encoded returns no files.
vectorcraft_doc::cancel::check()?;
let names: Vec<&str> = out.kept.iter().map(|(n, _)| n.as_str()).collect();
if o.zip {
let zip = super::zip::deflate(&out.kept, super::zip::FIXED_TIME).map_err(|e| bad(C, e))?;
let zip = super::zip::deflate(&out.kept, super::zip::FIXED_TIME, vectorcraft_doc::cancel::is_cancelled);
// A cancel that came while the files were deflated returns and writes no archive.
vectorcraft_doc::cancel::check()?;
let zip = zip.map_err(|e| bad(C, e))?;
let zip_name = safe_file_name(&format!("{}{}.zip", naming.prefix, naming.filename));
return match folder {
Some(dir) => {
let path = format!("{dir}/{zip_name}");
vectorcraft_doc::cancel::check()?;
writer(&path, &zip)?;
Ok(json!({ "path": path, "bytes": zip.len(), "files": names, "warnings": warnings }))
}
+55 -2
View File
@@ -662,7 +662,7 @@ fn zip_archives_deflate_what_gets_smaller() {
.collect();
let text = "<rect x=\"5\" y=\"5\" width=\"20\" height=\"20\"/>\n".repeat(100).into_bytes();
let files = [("art.svg", text.clone()), ("sub/ünï.png", noise.clone()), ("empty", vec![])];
let zip = super::zip::deflate(&files, super::zip::FIXED_TIME).unwrap();
let zip = super::zip::deflate(&files, super::zip::FIXED_TIME, || false).unwrap();
assert_eq!(methods(&zip), [8, 0, 0], "the text deflated, the noise and the empty file stored");
assert!(zip.windows(noise.len()).any(|w| w == noise.as_slice()), "a stored file's bytes are in the archive as they are");
assert!(zip.len() < super::zip::store(&files).unwrap().len());
@@ -671,7 +671,60 @@ fn zip_archives_deflate_what_gets_smaller() {
for ((n, b), (m, c)) in files.iter().zip(&back) {
assert_eq!((*n, b), (m.as_str(), c));
}
assert_eq!(super::zip::deflate(&files, super::zip::FIXED_TIME).unwrap(), zip, "the same files, the same archive");
assert_eq!(super::zip::deflate(&files, super::zip::FIXED_TIME, || false).unwrap(), zip, "the same files, the same archive");
}
/// `deflate` asks `stop` before each chunk of a file and drops the archive when it answers true.
#[test]
fn zip_deflating_stops_between_chunks() {
let big = vec![7u8; 3 * super::zip::DEFLATE_CHUNK];
let files = [("a.bin", big.clone()), ("b.bin", big)];
let asked = std::cell::Cell::new(0);
let all = super::zip::deflate(&files, super::zip::FIXED_TIME, || {
asked.set(asked.get() + 1);
false
});
assert!(all.is_ok());
assert_eq!(asked.get(), 6, "once per chunk of each file");
asked.set(0);
let r = super::zip::deflate(&files, super::zip::FIXED_TIME, || {
asked.set(asked.get() + 1);
asked.get() == 2
});
assert!(r.is_err(), "stopped");
assert_eq!(asked.get(), 2, "nothing more is deflated after the stop");
}
/// A cancel that comes after the last file was encoded (here: during the last step, a row that
/// repeats the one before it and writes nothing) returns no files and no archive, as it writes no
/// archive into a folder.
#[cfg(not(target_arch = "wasm32"))]
#[test]
fn a_cancel_after_the_last_file_returns_nothing() {
let mut s = session(1);
let dir = std::env::temp_dir().join(format!("vc-screens-cancel-last-{}", std::process::id()));
let folder = dir.to_string_lossy().replace(std::path::MAIN_SEPARATOR, "/");
let rows = json!([{"format": "svg"}, {"format": "svg"}]);
for p in [json!({ "formats": rows }), json!({ "formats": rows, "zip": true }), json!({ "formats": rows, "zip": true, "folder": folder })] {
let prepared = s.prepare_screen_export("document.exportForScreens", &p).unwrap();
let token = vectorcraft_doc::cancel::CancelToken::new();
let on_last = token.clone();
let sink: vectorcraft_doc::cancel::ProgressSink = std::rc::Rc::new(move |done, total, _| {
if done + 1 == total {
on_last.cancel();
}
});
let mut written = vec![];
let r = vectorcraft_doc::cancel::scope(&token, Some(sink), || {
prepared.write(&mut |path: &str, _: &[u8]| {
written.push(path.to_string());
Ok(())
})
});
assert!(matches!(r, Err(crate::EngineError::Cancelled)), "{p}: {r:?}");
assert_eq!(written, Vec::<String>::new(), "{p}");
}
let _ = std::fs::remove_dir_all(dir);
}
#[test]
+11 -3
View File
@@ -17,6 +17,8 @@ pub const FIXED_TIME: (u16, u16) = (0, DOS_DATE);
const UTF8_NAMES: u16 = 1 << 11;
/// Version needed to extract: 2.0.
const VERSION: u16 = 20;
/// How many bytes of a file [`deflate`] compresses between two calls of its `stop`: 1 MiB.
pub const DEFLATE_CHUNK: usize = 1 << 20;
/// The archive of `files` (`(name, bytes)`; `/` separates folders in a name), stored without
/// compression. Fails past the limits of a ZIP without its 64-bit extension (65535 files, 4 GB).
@@ -74,8 +76,9 @@ pub fn store<N: AsRef<str>, B: AsRef<[u8]>>(files: &[(N, B)]) -> Result<Vec<u8>,
/// The archive of `files` (`(name, bytes)`; `/` separates folders in a name), each deflated, or
/// stored when deflating doesn't make it smaller, every entry dated `(time, date)` (MS-DOS format).
/// Fails past the same limits as [`store`].
pub fn deflate<N: AsRef<str>, B: AsRef<[u8]>>(files: &[(N, B)], (time, date): (u16, u16)) -> Result<Vec<u8>, String> {
/// Fails past the same limits as [`store`]. `stop` is asked before each [`DEFLATE_CHUNK`] bytes are
/// deflated; when it answers true, the archive is dropped and an error returned.
pub fn deflate<N: AsRef<str>, B: AsRef<[u8]>>(files: &[(N, B)], (time, date): (u16, u16), stop: impl Fn() -> bool) -> Result<Vec<u8>, String> {
let too_big = || "too much to zip: at most 65535 files and 4 GB".to_string();
let (mut out, mut central) = (Vec::new(), Vec::new());
for (name, bytes) in files {
@@ -83,7 +86,12 @@ pub fn deflate<N: AsRef<str>, B: AsRef<[u8]>>(files: &[(N, B)], (time, date): (u
let mut crc = Crc::new();
crc.update(bytes);
let mut enc = flate2::write::DeflateEncoder::new(Vec::new(), flate2::Compression::default());
enc.write_all(bytes).map_err(|e| e.to_string())?;
for chunk in bytes.chunks(DEFLATE_CHUNK) {
if stop() {
return Err("stopped before the archive was finished".into());
}
enc.write_all(chunk).map_err(|e| e.to_string())?;
}
let deflated = enc.finish().map_err(|e| e.to_string())?;
let (method, data): (u16, &[u8]) = if deflated.len() < bytes.len() { (8, &deflated) } else { (0, bytes) };
let offset = u32::try_from(out.len()).map_err(|_| too_big())?;
+1 -1
View File
@@ -551,7 +551,7 @@ fn dos_time() -> (u16, u16) {
/// A zip archive of `files` ((path with `/` separators, bytes)), each deflated, or stored when
/// that isn't smaller, every entry dated now.
pub(crate) fn zip(files: &[(String, &[u8])]) -> Result<Vec<u8>> {
super::fileio::zip::deflate(files, dos_time()).map_err(|_| bad(C, "too large for a zip archive: package to a folder"))
super::fileio::zip::deflate(files, dos_time(), || false).map_err(|_| bad(C, "too large for a zip archive: package to a folder"))
}
#[cfg(test)]