From 5c506b9d2bcd587eb18032ce888316c58eb077e3 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 27 Sep 2026 22:05:58 -0700 Subject: [PATCH] List what a hold export can't read instead of skipping it (LH-12) An item the hold covers whose stored record or content can't be read goes in exceptions.csv with the path it would have had and the reason, rather than being left out silently. The file is always in the ZIP, so a header-only one shows nothing was missed, and manifest.sha256 carries its hash beside the manifest's. --- crates/jmap/src/inbuxa/hold_export.rs | 146 ++++++++++++++++++++++---- tests/src/system/legal_hold.rs | 8 +- 2 files changed, 133 insertions(+), 21 deletions(-) diff --git a/crates/jmap/src/inbuxa/hold_export.rs b/crates/jmap/src/inbuxa/hold_export.rs index be359b5..94093a0 100644 --- a/crates/jmap/src/inbuxa/hold_export.rs +++ b/crates/jmap/src/inbuxa/hold_export.rs @@ -9,6 +9,9 @@ //! calendars, contacts and files, and the deleted items the hold keeps, each //! with its SHA-256 in `manifest.csv`, and the manifest's own hash beside //! it. The hold's date range applies as it does to what's kept (LH-3). +//! Anything the hold covers that can't be read goes in `exceptions.csv` +//! with the reason, never silently left out; the file is always there, so an +//! empty one says nothing was missed. use common::{Server, hold::kept_member}; use email::{ @@ -52,6 +55,21 @@ struct Entry { sha256: String, } +/// One line of `exceptions.csv`: an item the hold covers that couldn't be +/// read, where it would have gone and why. +struct Missing { + path: String, + account: String, + kind: &'static str, + folder: String, + date: Option, + archived: bool, + reason: &'static str, +} + +const NO_BLOB: &str = "content not found in the blob store"; +const NO_RECORD: &str = "stored record not found"; + fn hex(bytes: &[u8]) -> String { bytes.iter().map(|b| format!("{b:02x}")).collect() } @@ -82,6 +100,7 @@ fn date_text(at: Option) -> String { struct Builder { zip: ZipWriter>>, entries: Vec, + missing: Vec, written: u64, } @@ -90,6 +109,7 @@ impl Builder { Builder { zip: ZipWriter::new(Cursor::new(Vec::new())), entries: Vec::new(), + missing: Vec::new(), written: 0, } } @@ -144,8 +164,31 @@ impl Builder { Ok(()) } - /// Closes the ZIP with its manifest and the manifest's hash. Returns the - /// bytes and how many items went in. + /// Records an item the hold covers that couldn't be read. + #[allow(clippy::too_many_arguments)] + fn missing( + &mut self, + path: String, + account: &str, + kind: &'static str, + folder: &str, + date: Option, + archived: bool, + reason: &'static str, + ) { + self.missing.push(Missing { + path, + account: account.to_string(), + kind, + folder: folder.to_string(), + date, + archived, + reason, + }); + } + + /// Closes the ZIP with its manifest, the exceptions and both hashes. + /// Returns the bytes and how many items went in. fn finish(mut self) -> trc::Result<(Vec, usize)> { let mut manifest = String::from("path,account,kind,folder,date,archived,size,sha256\n"); for e in &self.entries { @@ -161,7 +204,21 @@ impl Builder { e.sha256 )); } + let mut exceptions = String::from("path,account,kind,folder,date,archived,reason\n"); + for m in &self.missing { + exceptions.push_str(&format!( + "{},{},{},{},{},{},{}\n", + csv(&m.path), + csv(&m.account), + m.kind, + csv(&m.folder), + date_text(m.date), + m.archived, + csv(m.reason) + )); + } let manifest_hash = hex(&Sha256::digest(manifest.as_bytes())); + let exceptions_hash = hex(&Sha256::digest(exceptions.as_bytes())); let options = SimpleFileOptions::default().compression_method(CompressionMethod::Deflated); let fail = |err: zip::result::ZipError| { trc::StoreEvent::UnexpectedError @@ -171,9 +228,11 @@ impl Builder { }; self.zip.start_file("manifest.csv", options).map_err(fail)?; self.zip.write_all(manifest.as_bytes()).map_err(|e| fail(e.into()))?; + self.zip.start_file("exceptions.csv", options).map_err(fail)?; + self.zip.write_all(exceptions.as_bytes()).map_err(|e| fail(e.into()))?; self.zip.start_file("manifest.sha256", options).map_err(fail)?; self.zip - .write_all(format!("{manifest_hash} manifest.csv\n").as_bytes()) + .write_all(format!("{manifest_hash} manifest.csv\n{exceptions_hash} exceptions.csv\n").as_bytes()) .map_err(|e| fail(e.into()))?; let items = self.entries.len(); let bytes = self.zip.finish().map_err(fail)?.into_inner(); @@ -234,6 +293,19 @@ pub async fn build(server: &Server, hold: &Hold, asked: &[u32]) -> trc::Result<( .await .caused_by(trc::location!())?; for message in cache.emails.items.iter() { + let mail_path = |folder: &str| { + format!( + "{base}mail/{}/{}.eml", + folder.split('/').map(segment).collect::>().join("/"), + Id::from(message.document_id) + ) + }; + let folder = message + .mailboxes + .first() + .and_then(|m| cache.mailboxes.items.iter().find(|b| b.document_id == m.mailbox_id)) + .map(|b| b.path.clone()) + .unwrap_or_default(); let Some(metadata_) = data .get_value::>(ValueKey::property( account_id, @@ -243,6 +315,8 @@ pub async fn build(server: &Server, hold: &Hold, asked: &[u32]) -> trc::Result<( )) .await? else { + // No date to check against the hold's range, so it's listed + out.missing(mail_path(&folder), &address, "email", &folder, None, false, NO_RECORD); continue; }; let metadata = metadata_ @@ -252,20 +326,20 @@ pub async fn build(server: &Server, hold: &Hold, asked: &[u32]) -> trc::Result<( if !keeping.covers(Some(received)) { continue; } - let folder = message - .mailboxes - .first() - .and_then(|m| cache.mailboxes.items.iter().find(|b| b.document_id == m.mailbox_id)) - .map(|b| b.path.clone()) - .unwrap_or_default(); let hash = types::blob_hash::BlobHash::from(&metadata.blob_hash); - if let Some(bytes) = blob(server, hash.as_slice()).await? { - let path = format!( - "{base}mail/{}/{}.eml", - folder.split('/').map(segment).collect::>().join("/"), - Id::from(message.document_id) - ); - out.add(path, &bytes, &address, "email", &folder, Some(received as i64), false)?; + match blob(server, hash.as_slice()).await? { + Some(bytes) => { + out.add(mail_path(&folder), &bytes, &address, "email", &folder, Some(received as i64), false)? + } + None => out.missing( + mail_path(&folder), + &address, + "email", + &folder, + Some(received as i64), + false, + NO_BLOB, + ), } } @@ -315,6 +389,7 @@ pub async fn build(server: &Server, hold: &Hold, asked: &[u32]) -> trc::Result<( )) .await? else { + out.missing(zip_path(Some(".ics")), &address, kind, folder, Some(*start), false, NO_RECORD); continue; }; let event = event_.unarchive::().caused_by(trc::location!())?; @@ -330,6 +405,7 @@ pub async fn build(server: &Server, hold: &Hold, asked: &[u32]) -> trc::Result<( )) .await? else { + out.missing(zip_path(Some(".vcf")), &address, kind, folder, None, false, NO_RECORD); continue; }; let card = card_.unarchive::().caused_by(trc::location!())?; @@ -346,15 +422,18 @@ pub async fn build(server: &Server, hold: &Hold, asked: &[u32]) -> trc::Result<( )) .await? else { + out.missing(zip_path(None), &address, kind, folder, None, false, NO_RECORD); continue; }; let file = file_.unarchive::().caused_by(trc::location!())?; let Some(props) = file.file.as_ref() else { + out.missing(zip_path(None), &address, kind, folder, None, false, NO_RECORD); continue; }; let hash = types::blob_hash::BlobHash::from(&props.blob_hash); - if let Some(bytes) = blob(server, hash.as_slice()).await? { - out.add(zip_path(None), &bytes, &address, kind, folder, None, false)?; + match blob(server, hash.as_slice()).await? { + Some(bytes) => out.add(zip_path(None), &bytes, &address, kind, folder, None, false)?, + None => out.missing(zip_path(None), &address, kind, folder, None, false, NO_BLOB), } } _ => {} @@ -388,8 +467,10 @@ pub async fn build(server: &Server, hold: &Hold, asked: &[u32]) -> trc::Result<( ArchivedItem::SieveScript(s) => format!("{}{ext}", segment(&s.name)), _ => format!("{id}{ext}"), }; - if let Some(bytes) = blob(server, item.blob_id().hash.as_slice()).await? { - out.add(format!("{base}archived/{kind}/{name}"), &bytes, &address, kind, "", date, true)?; + let path = format!("{base}archived/{kind}/{name}"); + match blob(server, item.blob_id().hash.as_slice()).await? { + Some(bytes) => out.add(path, &bytes, &address, kind, "", date, true)?, + None => out.missing(path, &address, kind, "", date, true, NO_BLOB), } } } @@ -425,5 +506,30 @@ mod tests { let mut hash = String::new(); std::io::Read::read_to_string(&mut zip.by_name("manifest.sha256").unwrap(), &mut hash).unwrap(); assert!(hash.starts_with(&hex(&Sha256::digest(manifest.as_bytes())))); + // Nothing missed, and the file says so + let mut exceptions = String::new(); + std::io::Read::read_to_string(&mut zip.by_name("exceptions.csv").unwrap(), &mut exceptions).unwrap(); + assert_eq!(exceptions, "path,account,kind,folder,date,archived,reason\n"); + } + + #[test] + fn what_cant_be_read_is_listed_not_dropped() { + let mut b = Builder::new(); + b.add("a@example.com/mail/INBOX/1.eml".into(), b"Subject: x\r\n\r\ny", "a@example.com", "email", "INBOX", Some(0), false) + .unwrap(); + b.missing("a@example.com/mail/INBOX/2.eml".into(), "a@example.com", "email", "INBOX", Some(0), false, NO_BLOB); + let (bytes, items) = b.finish().unwrap(); + assert_eq!(items, 1, "a missing item isn't counted as collected"); + let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).unwrap(); + assert!(zip.by_name("a@example.com/mail/INBOX/2.eml").is_err()); + let mut exceptions = String::new(); + std::io::Read::read_to_string(&mut zip.by_name("exceptions.csv").unwrap(), &mut exceptions).unwrap(); + assert!( + exceptions.contains("a@example.com/mail/INBOX/2.eml,a@example.com,email,INBOX,") && exceptions.contains(NO_BLOB), + "{exceptions}" + ); + let mut hash = String::new(); + std::io::Read::read_to_string(&mut zip.by_name("manifest.sha256").unwrap(), &mut hash).unwrap(); + assert!(hash.contains(&format!("{} exceptions.csv", hex(&Sha256::digest(exceptions.as_bytes()))))); } } diff --git a/tests/src/system/legal_hold.rs b/tests/src/system/legal_hold.rs index 621fa45..05916d0 100644 --- a/tests/src/system/legal_hold.rs +++ b/tests/src/system/legal_hold.rs @@ -494,7 +494,7 @@ pub async fn test(test: &mut TestServer) { assert!( names .iter() - .all(|n| n.starts_with("frozen@example.com/") || n.starts_with("manifest.")), + .all(|n| n.starts_with("frozen@example.com/") || n.starts_with("manifest.") || n == "exceptions.csv"), "LH-12: an account the hold doesn't cover was exported: {names:?}" ); let mut manifest = String::new(); @@ -508,6 +508,12 @@ pub async fn test(test: &mut TestServer) { .collect(); assert!(hash.starts_with(&expected), "LH-12: the manifest's hash doesn't match"); assert!(manifest.contains(",true,"), "LH-12: nothing marked archived: {manifest}"); + let mut exceptions = String::new(); + std::io::Read::read_to_string(&mut zip.by_name("exceptions.csv").unwrap(), &mut exceptions).unwrap(); + assert_eq!( + exceptions, "path,account,kind,folder,date,archived,reason\n", + "LH-12: items the hold covers couldn't be read" + ); // LH-13: only sysLegalHoldExport starts one let (_, response) = frozen .hold_call(