From d814ba707fcb70b9b9565ba10b96ec320ea2d3df Mon Sep 17 00:00:00 2001
From: "C. Titus Brown" <titus@idyll.org>
Date: Sun, 15 Dec 2024 04:00:59 -0800
Subject: [PATCH] MRG: propagate zipfile errors (#3431)

This PR switches from `ZipStorageBuilder` to `ZipStorageTryBuilder` in
order to propagate errors from bad zip files.

Fixes https://github.com/sourmash-bio/sourmash/issues/3430
---
 Cargo.lock                  |  2 +-
 include/sourmash.h          |  1 +
 src/core/Cargo.toml         |  2 +-
 src/core/src/errors.rs      |  6 ++++++
 src/core/src/storage/mod.rs | 10 +++++-----
 5 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 9c45cfc7c..853a3f9eb 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1629,7 +1629,7 @@ checksum = "9f1341053f34bb13b5e9590afb7d94b48b48d4b87467ec28e3c238693bb553de"
 
 [[package]]
 name = "sourmash"
-version = "0.17.2"
+version = "0.18.0"
 dependencies = [
  "az",
  "byteorder",
diff --git a/include/sourmash.h b/include/sourmash.h
index beb64ea45..f650035de 100644
--- a/include/sourmash.h
+++ b/include/sourmash.h
@@ -49,6 +49,7 @@ enum SourmashErrorCode {
   SOURMASH_ERROR_CODE_NIFFLER_ERROR = 100005,
   SOURMASH_ERROR_CODE_CSV_ERROR = 100006,
   SOURMASH_ERROR_CODE_ROCKS_DB_ERROR = 100007,
+  SOURMASH_ERROR_CODE_ZIP_ERROR = 100008,
 };
 typedef uint32_t SourmashErrorCode;
 
diff --git a/src/core/Cargo.toml b/src/core/Cargo.toml
index 41cf944fa..a45d03481 100644
--- a/src/core/Cargo.toml
+++ b/src/core/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "sourmash"
-version = "0.17.2"
+version = "0.18.0"
 authors = ["Luiz Irber <luiz@sourmash.bio>", "N. Tessa Pierce-Ward <tessa@sourmash.bio>", "C. Titus Brown <titus@idyll.org>"]
 description = "tools for comparing biological sequences with k-mer sketches"
 repository = "https://github.com/sourmash-bio/sourmash"
diff --git a/src/core/src/errors.rs b/src/core/src/errors.rs
index 30d269a13..e8ce3e68a 100644
--- a/src/core/src/errors.rs
+++ b/src/core/src/errors.rs
@@ -90,6 +90,9 @@ pub enum SourmashError {
     #[cfg(feature = "branchwater")]
     #[error(transparent)]
     RocksDBError(#[from] rocksdb::Error),
+
+    #[error(transparent)]
+    ZipError(#[from] piz::result::ZipError),
 }
 
 #[derive(Debug, Error)]
@@ -140,6 +143,7 @@ pub enum SourmashErrorCode {
     NifflerError = 100_005,
     CsvError = 100_006,
     RocksDBError = 100_007,
+    ZipError = 100_008,
 }
 
 #[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))]
@@ -179,6 +183,8 @@ impl SourmashErrorCode {
             #[cfg(not(target_arch = "wasm32"))]
             #[cfg(feature = "branchwater")]
             SourmashError::RocksDBError { .. } => SourmashErrorCode::RocksDBError,
+
+            SourmashError::ZipError { .. } => SourmashErrorCode::ZipError,
         }
     }
 }
diff --git a/src/core/src/storage/mod.rs b/src/core/src/storage/mod.rs
index e098f58eb..b026b80d4 100644
--- a/src/core/src/storage/mod.rs
+++ b/src/core/src/storage/mod.rs
@@ -403,22 +403,22 @@ impl ZipStorage {
         let zip_file = File::open(location.as_ref())?;
         let mapping = unsafe { memmap2::Mmap::map(&zip_file)? };
 
-        let mut storage = ZipStorageBuilder {
+        let mut storage = ZipStorageTryBuilder {
             mapping: Some(mapping),
             archive_builder: |mapping: &Option<memmap2::Mmap>| {
-                piz::ZipArchive::new(mapping.as_ref().unwrap()).unwrap()
+                piz::ZipArchive::new(mapping.as_ref().unwrap())
             },
             metadata_builder: |archive: &piz::ZipArchive| {
-                archive
+                Ok(archive
                     .entries()
                     .iter()
                     .map(|entry| (entry.path.as_os_str(), entry))
-                    .collect()
+                    .collect())
             },
             subdir: None,
             path: Some(location.as_ref().into()),
         }
-        .build();
+        .try_build()?;
 
         let subdir = find_subdirs(storage.borrow_archive())?;
         storage.with_mut(|fields| *fields.subdir = subdir);