craft-idml, craft-epub: read and write through craft-zip (#950)

* craft-zip: ZIP preflight, bounded entry read and Budget (from craft-archive, #129)

The ZIP half of the craft-archive proposal (#126, #129): an allocation-free check of the end
records and central directory that runs before a ZIP reader indexes the archive, a bounded
entry read that distrusts the declared size, and the named Budget both charge. The XML half
of #129 is not carried: craft-xml bounds XML now.

* craft-zip: Archive (entry-count guard, duplicate names, budgeted reads) and Writer

check_entry_count is craft-opc's guard: the declared entry count, ZIP64 included, checked on
the raw bytes before zip parses the central directory. Archive reads entries within a per-entry
limit and a total Budget through read_entry. Duplicate names are found by counting the central
directory headers against the entries zip kept. Writer is craft-opc's ZipOut: stored or deflated
entries, ZIP64 for large ones, one fixed timestamp.

* craft-idml, craft-epub: read and write through craft-zip

Both crates drop their own bounded ZIP reads and zip writers. They gain the entry-count check
before the archive is indexed (ZIP64 included) and archives whose bytes depend only on their
content (one fixed timestamp).

---------

Co-authored-by: skylar taylor-barrick <hi@skylar.cc>

Cargo.lock

@@ -1025,7 +1025,7 @@ name = "craft-epub"
 version = "0.1.0"
 dependencies = [
  "craft-xml",
- "flate2",
+ "craft-zip",
  "thiserror 2.0.21",
  "zip",
 ]
@@ -1185,7 +1185,7 @@ name = "craft-idml"
 version = "0.1.0"
 dependencies = [
  "craft-xml",
- "flate2",
+ "craft-zip",
  "thiserror 2.0.21",
  "zip",
 ]

crates/epub/Cargo.toml

@@ -13,8 +13,9 @@ workspace = true
 [dependencies]
 craft-xml = { workspace = true }
 thiserror = { workspace = true }
-# `deflate-flate2` with flate2's default miniz_oxide backend, as craft-update pins it (zip's own
-# `deflate` feature would switch a whole app's flate2 to zlib-rs).
+craft-zip = { workspace = true }
+
+[dev-dependencies]
+# The tests build hostile archives by hand.
 zip = { version = "9", default-features = false, features = ["deflate-flate2"] }
-# The backend of zip's deflate: `deflate-flate2` leaves the choice to the build.
-flate2 = { version = "1", default-features = false, features = ["rust_backend"] }
+craft-zip = { workspace = true, features = ["test-support"] }

crates/epub/README.md

@@ -94,7 +94,8 @@ for path in &spine {
 | `max_total_bytes` (all entries read from one package) | 256 MiB | `Error::TooLarge` |
 | `max_manifest`, `max_spine` | 10,000 each | further items are ignored |
 
-Sizes are checked against what an entry really expands to, not what the archive declares. The two
+The ZIP layer is craft-zip: the declared entry count is checked before the archive is indexed, and
+sizes are checked against what an entry really expands to, not what the archive declares. The two
 XML documents are read with craft-xml's limits (depth 256, 2,000,000 nodes) and `max_entry_bytes`.
 
 ## Where it came from

crates/epub/src/read.rs

@@ -1,7 +1,6 @@
 //! An EPUB 2 or 3 package, read: container, package document, and bounded entry reads.
 
 use std::collections::HashMap;
-use std::io::{Cursor, Read};
 
 use craft_xml::{El, Local, Options};
 
@@ -224,10 +223,8 @@ pub fn parse_package(xml: &[u8], opf_dir: &str, limits: &Limits) -> Result<Packa
 /// An open EPUB package. Entries are decompressed only when asked for, within the byte budgets
 /// of its [`Limits`].
 pub struct Epub<'a> {
-    zip: zip::ZipArchive<Cursor<&'a [u8]>>,
+    zip: craft_zip::Archive<'a>,
     limits: Limits,
-    /// Bytes the entries still to be read may expand to.
-    left: u64,
     /// The archive path of the package document.
     pub opf_path: String,
     pub package: Package,
@@ -242,11 +239,16 @@ impl<'a> Epub<'a> {
     /// Reads the container and the package document. The `mimetype` entry is not required:
     /// packages without one, or with it compressed, are common.
     pub fn open_with(bytes: &'a [u8], limits: Limits) -> Result<Self, Error> {
-        let zip = zip::ZipArchive::new(Cursor::new(bytes)).map_err(|e| Error::NotEpub(e.to_string()))?;
-        if zip.len() > limits.max_entries {
-            return Err(Error::TooManyEntries { found: zip.len(), limit: limits.max_entries });
-        }
-        let mut epub = Epub { zip, limits, left: limits.max_total_bytes, opf_path: String::new(), package: Package::default() };
+        let zip =
+            craft_zip::Archive::open(bytes, craft_zip::Limits { max_entries: limits.max_entries as u64, max_total_bytes: limits.max_total_bytes })
+                .map_err(|e| match e {
+                    craft_zip::Error::TooManyEntries { declared, limit } => Error::TooManyEntries {
+                        found: usize::try_from(declared).unwrap_or(usize::MAX),
+                        limit: usize::try_from(limit).unwrap_or(usize::MAX),
+                    },
+                    e => Error::NotEpub(e.to_string()),
+                })?;
+        let mut epub = Epub { zip, limits, opf_path: String::new(), package: Package::default() };
         let container = epub.read("META-INF/container.xml")?.ok_or_else(|| Error::NotEpub("META-INF/container.xml is missing".into()))?;
         let opf_path = parse_container(&container)?.ok_or_else(|| Error::NotEpub("container.xml names no package document".into()))?;
         // The container gives a path from the archive's root; resolving it also drops a leading `/`.
@@ -262,28 +264,20 @@ impl<'a> Epub<'a> {
 
     /// Does the archive have an entry `path`?
     pub fn contains(&self, path: &str) -> bool {
-        self.zip.index_for_name(path).is_some()
+        self.zip.contains(path)
     }
 
     /// The bytes of archive entry `path`, or `None` when the archive has no such entry. An entry
     /// past the limits is an error, and so is one that expands to more than it declared.
     pub fn read(&mut self, path: &str) -> Result<Option<Vec<u8>>, Error> {
-        let Some(index) = self.zip.index_for_name(path).filter(|_| !path.ends_with('/')) else { return Ok(None) };
-        let part = |msg: String| Error::Part { part: path.to_string(), msg };
-        let f = self.zip.by_index(index).map_err(|e| part(e.to_string()))?;
-        let limit = self.limits.max_entry_bytes.min(self.left);
-        let too_large = || Error::TooLarge { part: path.to_string(), limit };
-        if f.size() > limit {
-            return Err(too_large());
+        if path.ends_with('/') {
+            return Ok(None);
         }
-        let mut buf = Vec::new();
-        f.take(limit.saturating_add(1)).read_to_end(&mut buf).map_err(|e| part(e.to_string()))?;
-        let n = buf.len() as u64;
-        if n > limit {
-            return Err(too_large());
-        }
-        self.left = self.left.saturating_sub(n);
-        Ok(Some(buf))
+        let limit = self.limits.max_entry_bytes.min(self.zip.budget().remaining());
+        self.zip.read(path, self.limits.max_entry_bytes).map_err(|e| match e {
+            craft_zip::Error::EntryTooLarge { .. } | craft_zip::Error::BudgetSpent { .. } => Error::TooLarge { part: path.to_string(), limit },
+            e => Error::Part { part: path.to_string(), msg: e.to_string() },
+        })
     }
 
     /// [`Epub::read`] for what a file of the package refers to: `href` as written in the file at
@@ -295,6 +289,6 @@ impl<'a> Epub<'a> {
 
     /// Bytes the entries still to be read may expand to.
     pub fn budget_left(&self) -> u64 {
-        self.left
+        self.zip.budget().remaining()
     }
 }

crates/epub/src/write.rs

@@ -3,7 +3,6 @@
 use std::borrow::Cow;
 use std::collections::HashSet;
 use std::fmt::Write as _;
-use std::io::Write as _;
 
 use craft_xml::{esc, esc_attr, is_name};
 
@@ -316,10 +315,8 @@ pub fn write(book: &Book) -> Result<Vec<u8>, Error> {
         w3cdtf(&book.metadata.modified).ok_or_else(|| Error::Invalid(format!("`{}` is not a modification date", book.metadata.modified)))?;
     let opf = package_document(book, &modified);
     let nav = nav_document(book);
-    let fail = |e: &dyn std::fmt::Display| Error::Invalid(format!("the archive could not be written: {e}"));
-    let mut z = zip::ZipWriter::new(std::io::Cursor::new(Vec::new()));
-    let stored = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored);
-    let deflated = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated);
+    let fail = |e: craft_zip::Error| Error::Invalid(e.to_string());
+    let mut z = craft_zip::Writer::new();
     let own = [
         ("mimetype".to_string(), MIMETYPE.as_bytes(), true),
         ("META-INF/container.xml".to_string(), CONTAINER.as_bytes(), false),
@@ -328,10 +325,7 @@ pub fn write(book: &Book) -> Result<Vec<u8>, Error> {
     ];
     let resources = book.resources.iter().map(|r| (format!("{ROOT}/{}", r.path), r.data.as_ref(), r.stored));
     for (name, data, store) in own.into_iter().chain(resources) {
-        // More than 4 GiB needs the ZIP64 form.
-        let options = if store { stored } else { deflated }.large_file(data.len() >= u32::MAX as usize);
-        z.start_file(name, options).map_err(|e| fail(&e))?;
-        z.write_all(data).map_err(|e| fail(&e))?;
+        z.add(&name, data, if store { craft_zip::Compression::Stored } else { craft_zip::Compression::Deflated }).map_err(fail)?;
     }
-    z.finish().map(std::io::Cursor::into_inner).map_err(|e| fail(&e))
+    z.finish().map_err(fail)
 }

crates/epub/tests/read.rs

@@ -252,3 +252,11 @@ fn a_package_document_past_the_entry_limit_is_refused() {
     let limits = Limits { max_entry_bytes: (xml.len() - 1) as u64, ..Limits::DEFAULT };
     assert!(matches!(parse_package(&xml, "", &limits), Err(Error::Part { .. })));
 }
+
+/// A ZIP64 end record can declare millions of entries in a small file; the count is refused on
+/// the raw bytes, before the archive is indexed.
+#[test]
+fn an_archive_declaring_too_many_entries_is_refused_before_it_is_indexed() {
+    let bomb = craft_zip::zip64_entry_count_sample(100_001);
+    assert_eq!(Epub::open(&bomb).err(), Some(Error::TooManyEntries { found: 100_001, limit: 100_000 }));
+}

crates/idml/Cargo.toml

@@ -13,8 +13,9 @@ workspace = true
 [dependencies]
 craft-xml = { workspace = true }
 thiserror = { workspace = true }
-# `deflate-flate2` with flate2's default miniz_oxide backend, as craft-update pins it (zip's own
-# `deflate` feature would switch a whole app's flate2 to zlib-rs).
+craft-zip = { workspace = true }
+
+[dev-dependencies]
+# The tests build hostile archives by hand.
 zip = { version = "9", default-features = false, features = ["deflate-flate2"] }
-# The backend of zip's deflate: `deflate-flate2` leaves the choice to the build.
-flate2 = { version = "1", default-features = false, features = ["rust_backend"] }
+craft-zip = { workspace = true, features = ["test-support"] }

crates/idml/README.md

@@ -53,7 +53,8 @@ for line in &doc.warnings { /* a part designmap.xml lists and the package lacks
 | `xml.max_depth` | 256 | `Error::Part` |
 | `xml.max_nodes`, `xml.max_attrs_total` (one part) | 8,000,000 and 32,000,000 | `Error::Part` |
 
-Sizes are checked against what a part really expands to, not what the archive declares.
+The ZIP layer is craft-zip: the declared entry count is checked before the archive is indexed, and
+sizes are checked against what a part really expands to, not what the archive declares.
 
 ## Writing
 

crates/idml/src/package.rs

@@ -1,7 +1,6 @@
 //! The ZIP container: bounded part reads, lenient part names, designmap includes, and the writer.
 
 use std::collections::HashMap;
-use std::io::{Cursor, Read, Write};
 
 use craft_xml::El;
 
@@ -34,12 +33,10 @@ impl Default for Limits {
 /// An IDML package whose parts are decompressed only when asked for, within byte budgets that
 /// don't trust the sizes the archive claims.
 pub struct Package<'a> {
-    zip: zip::ZipArchive<Cursor<&'a [u8]>>,
+    zip: craft_zip::Archive<'a>,
     /// Normalised part name → entry name, first entry wins (zip order, so the choice is stable).
     lenient: HashMap<String, String>,
     limits: Limits,
-    /// Bytes the parts still to be read may expand to.
-    left: u64,
 }
 
 /// `designmap.xml` with the parts it includes.
@@ -63,46 +60,40 @@ impl<'a> Package<'a> {
         Self::open_with(bytes, Limits::DEFAULT)
     }
 
+    /// The declared entry count is checked against [`Limits::max_entries`] before the archive
+    /// is indexed.
     pub fn open_with(bytes: &'a [u8], limits: Limits) -> Result<Self> {
-        let zip = zip::ZipArchive::new(Cursor::new(bytes)).map_err(|e| Error::NotIdml(e.to_string()))?;
-        if zip.len() > limits.max_entries {
-            return Err(Error::TooManyEntries { found: zip.len(), limit: limits.max_entries });
-        }
+        let zip =
+            craft_zip::Archive::open(bytes, craft_zip::Limits { max_entries: limits.max_entries as u64, max_total_bytes: limits.max_total_bytes })
+                .map_err(|e| match e {
+                    craft_zip::Error::TooManyEntries { declared, limit } => Error::TooManyEntries {
+                        found: usize::try_from(declared).unwrap_or(usize::MAX),
+                        limit: usize::try_from(limit).unwrap_or(usize::MAX),
+                    },
+                    e => Error::NotIdml(e.to_string()),
+                })?;
         let mut lenient: HashMap<String, String> = HashMap::new();
-        for i in 0..zip.len() {
-            let Some(Ok(name)) = zip.name_for_index(i) else { continue };
-            if name.ends_with('/') {
-                continue;
-            }
+        for name in zip.names().filter(|n| !n.ends_with('/')) {
             lenient.entry(normalize_part_name(&name)).or_insert_with(|| name.into_owned());
         }
-        Ok(Package { zip, lenient, limits, left: limits.max_total_bytes })
+        Ok(Package { zip, lenient, limits })
     }
 
     /// The bytes of part `name` (matched exactly, then leniently: writers differ in
     /// percent-encoding, case, path separators and a leading `./` or `/`), or `None` if the
     /// package has no such part.
     pub fn part(&mut self, name: &str) -> Result<Option<Vec<u8>>> {
-        let exact = self.zip.index_for_name(name).filter(|_| !name.ends_with('/'));
-        let Some(i) = exact.or_else(|| self.lenient.get(&normalize_part_name(name)).and_then(|n| self.zip.index_for_name(n))) else {
+        let exact = (self.zip.contains(name) && !name.ends_with('/')).then_some(name);
+        let Some(stored) = exact.or_else(|| self.lenient.get(&normalize_part_name(name)).map(String::as_str)) else {
             return Ok(None);
         };
-        let part = self.zip.name_for_index(i).and_then(|n| n.ok()).map_or_else(|| name.to_string(), |n| n.into_owned());
-        let f = self.zip.by_index(i).map_err(|e| Error::NotIdml(e.to_string()))?;
-        let limit = self.limits.max_part_bytes.min(self.left);
-        if f.size() > limit {
-            return Err(Error::TooLarge { part, limit });
-        }
-        let mut buf = Vec::new();
-        if let Err(e) = f.take(limit.saturating_add(1)).read_to_end(&mut buf) {
-            return Err(Error::Part { part, msg: e.to_string() });
-        }
-        let n = buf.len() as u64;
-        if n > limit {
-            return Err(Error::TooLarge { part, limit });
-        }
-        self.left = self.left.saturating_sub(n);
-        Ok(Some(buf))
+        let limit = self.limits.max_part_bytes.min(self.zip.budget().remaining());
+        let stored = stored.to_string();
+        self.zip.read(&stored, self.limits.max_part_bytes).map_err(|e| match e {
+            craft_zip::Error::EntryTooLarge { .. } | craft_zip::Error::BudgetSpent { .. } => Error::TooLarge { part: stored.clone(), limit },
+            craft_zip::Error::Entry { msg, .. } => Error::Part { part: stored.clone(), msg },
+            e => Error::NotIdml(e.to_string()),
+        })
     }
 
     /// Part `name` as an element tree ([`crate::parse_part`]).
@@ -114,7 +105,7 @@ impl<'a> Package<'a> {
 
     /// Bytes the parts still to be read may expand to.
     pub fn budget_left(&self) -> u64 {
-        self.left
+        self.zip.budget().remaining()
     }
 
     /// Reads `designmap.xml` and every part it includes.
@@ -173,19 +164,14 @@ fn normalize_part_name(name: &str) -> String {
 }
 
 /// Zips an IDML package: the stored `mimetype` first, then `parts` in order as (name, bytes).
+/// Equal parts give equal bytes.
 pub fn write_package<N: AsRef<str>, D: AsRef<[u8]>>(parts: &[(N, D)]) -> Result<Vec<u8>> {
-    use zip::write::SimpleFileOptions;
-    let fail = |e: &dyn std::fmt::Display| Error::Write(e.to_string());
-    let mut w = zip::ZipWriter::new(Cursor::new(Vec::new()));
-    let stored = SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored);
-    let deflated = SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated);
-    w.start_file("mimetype", stored).map_err(|e| fail(&e))?;
-    w.write_all(MIMETYPE.as_bytes()).map_err(|e| fail(&e))?;
+    use craft_zip::Compression;
+    let fail = |e: craft_zip::Error| Error::Write(e.to_string());
+    let mut w = craft_zip::Writer::new();
+    w.add("mimetype", MIMETYPE.as_bytes(), Compression::Stored).map_err(fail)?;
     for (name, data) in parts {
-        let data = data.as_ref();
-        // More than 4 GiB needs the ZIP64 form.
-        w.start_file(name.as_ref(), deflated.large_file(data.len() >= u32::MAX as usize)).map_err(|e| fail(&e))?;
-        w.write_all(data).map_err(|e| fail(&e))?;
+        w.add(name.as_ref(), data.as_ref(), Compression::Deflated).map_err(fail)?;
     }
-    w.finish().map(Cursor::into_inner).map_err(|e| fail(&e))
+    w.finish().map_err(fail)
 }

crates/idml/tests/package.rs

@@ -244,3 +244,16 @@ fn parts_are_written_one_element_per_line_without_touching_text() {
     );
     assert!(with_props(El::new("A"), Vec::new()).children.is_empty());
 }
+
+/// A ZIP64 end record can declare millions of entries in a small file; the count is refused on
+/// the raw bytes, before the archive is indexed.
+#[test]
+fn a_package_declaring_too_many_entries_is_refused_before_it_is_indexed() {
+    let bomb = craft_zip::zip64_entry_count_sample(100_001);
+    assert_eq!(Package::open(&bomb).err(), Some(Error::TooManyEntries { found: 100_001, limit: 100_000 }));
+}
+
+#[test]
+fn equal_parts_give_equal_bytes() {
+    assert_eq!(sample(), sample());
+}