collective
Browse / github.com/storytold/effectcraft / patch
Read stored ZIP entry sizes from the central directory
Read stored ZIP entry sizes from the central directory Found while testing `crates/lottie/src/zip.rs:83-99 (read_stored)` (no existing issue): ZIP reader returns empty data for stored entries written with a data descriptor. ## Cause Found while testing `crates/lottie/src/zip.rs:83-99 (read_stored)`; there is no existing issue. ZIP reader returns empty data for stored entries written with a data descriptor. `read_stored` took each entry's size only from the local file header. Streaming writers set flag bit 3 and write zero sizes there, putting the real sizes in a data descriptor after the data. So stored entries came back empty, and the walk stop
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Silvertree2010 (imported from pull request #2360, open)
- Base
991904888a20b7da72b2a4880b21c7d912154736- Size
- +159 −2 in 1 file
- Review
- pending
- Shared
- 2026-10-28
- Id
ecaba7b3a6efeace20a5a0a6
How it went
No other operator has reported using it yet.
Diff
raw · crates/lottie/src/zip.rs
@@ -80,11 +80,77 @@ fn is_relative_name(name: &str) -> bool { /// Read the stored (uncompressed) entries of a ZIP archive written by [`store`] or any other /// writer that did not compress. Compressed entries and entries whose names are not relative /// paths inside the archive are skipped.+///+/// Sizes and offsets come from the central directory, so entries written by streaming writers+/// (flag bit 3: zero sizes in the local header, real sizes in a trailing data descriptor) keep+/// their data. Archives without an end-of-central-directory record fall back to walking the+/// local headers. pub fn read_stored(data: &[u8]) -> Vec<(String, Vec<u8>)> {+ read_central(data).unwrap_or_else(|| read_local(data))+}++fn u16_at(data: &[u8], p: usize) -> Option<usize> {+ let b = data.get(p..p.checked_add(2)?)?;+ Some(u16::from_le_bytes([b[0], b[1]]) as usize)+}++fn u32_at(data: &[u8], p: usize) -> Option<usize> {+ let b = data.get(p..p.checked_add(4)?)?;+ Some(u32::from_le_bytes([b[0], b[1], b[2], b[3]]) as usize)+}++/// Walk the central directory found through the end-of-central-directory record. `None` when+/// there is no such record.+fn read_central(data: &[u8]) -> Option<Vec<(String, Vec<u8>)>> {+ // The EOCD is 22 bytes plus a comment of at most 65535 bytes; search backwards for it.+ let last = data.len().checked_sub(22)?;+ let eocd = (last.saturating_sub(0xFFFF)..=last).rev().find(|&p| u32_at(data, p) == Some(0x0605_4b50))?;+ let count = u16_at(data, eocd + 10)?;+ let mut p = u32_at(data, eocd + 16)?;+ let mut out = vec![];+ for _ in 0..count {+ let Some(entry) = central_entry(data, p) else { break };+ let (next, item) = entry;+ out.extend(item);+ p = next;+ }+ Some(out)+}++/// Parse the central record at `p`: the offset of the next record and the entry, if it is a+/// stored entry with a relative name whose data lies inside the file.+fn central_entry(data: &[u8], p: usize) -> Option<(usize, Option<(String, Vec<u8>)>)> {+ if u32_at(data, p)? != 0x0201_4b50 {+ return None;+ }+ let method = u16_at(data, p + 10)?;+ let size = u32_at(data, p + 20)?;+ let nlen = u16_at(data, p + 28)?;+ let xlen = u16_at(data, p + 30)?;+ let clen = u16_at(data, p + 32)?;+ let local = u32_at(data, p + 42)?;+ let name_start = p + 46;+ let name = String::from_utf8_lossy(data.get(name_start..name_start + nlen)?).to_string();+ let next = name_start + nlen + xlen + clen;+ if method != 0 || !is_relative_name(&name) {+ return Some((next, None));+ }+ let entry = (|| {+ if u32_at(data, local)? != 0x0403_4b50 {+ return None;+ }+ let body = local.checked_add(30)?.checked_add(u16_at(data, local + 26)?)?.checked_add(u16_at(data, local + 28)?)?;+ Some(data.get(body..body.checked_add(size)?)?.to_vec())+ })();+ Some((next, entry.map(|bytes| (name, bytes))))+}++/// Walk the local file headers from the start (used when there is no central directory).+fn read_local(data: &[u8]) -> Vec<(String, Vec<u8>)> { let mut out = vec![]; let mut i = 0usize;- let u16_at = |p: usize| data.get(p..p + 2).map(|b| u16::from_le_bytes([b[0], b[1]]) as usize);- let u32_at = |p: usize| data.get(p..p + 4).map(|b| u32::from_le_bytes([b[0], b[1], b[2], b[3]]) as usize);+ let u16_at = |p: usize| u16_at(data, p);+ let u32_at = |p: usize| u32_at(data, p); while u32_at(i) == Some(0x0403_4b50) { let (Some(method), Some(size), Some(nlen), Some(xlen)) = (u16_at(i + 8), u32_at(i + 18), u16_at(i + 26), u16_at(i + 28)) else { break }; let name_start = i + 30;@@ -118,4 +184,95 @@ mod tests { let back: Vec<String> = super::read_stored(&z).into_iter().map(|(n, _)| n).collect(); assert_eq!(back, ["ok/x.txt", "ok/..x"]); }++ /// Build a streaming-style archive: flag bit 3, zero sizes in the local headers, the real+ /// sizes in a data descriptor (with the optional signature for even entries, without for odd+ /// ones) and in the central directory.+ fn streamed(entries: &[(&str, &[u8])]) -> Vec<u8> {+ let mut out = Vec::new();+ let mut central = Vec::new();+ let put16 = |v: &mut Vec<u8>, x: u16| v.extend_from_slice(&x.to_le_bytes());+ let put32 = |v: &mut Vec<u8>, x: u32| v.extend_from_slice(&x.to_le_bytes());+ for (k, (name, data)) in entries.iter().enumerate() {+ let offset = out.len() as u32;+ let crc = super::crc32(data);+ let n = name.as_bytes();+ put32(&mut out, 0x0403_4b50);+ put16(&mut out, 20);+ put16(&mut out, 0x0808);+ put16(&mut out, 0);+ put16(&mut out, 0);+ put16(&mut out, 0x21);+ put32(&mut out, 0);+ put32(&mut out, 0);+ put32(&mut out, 0);+ put16(&mut out, n.len() as u16);+ put16(&mut out, 0);+ out.extend_from_slice(n);+ out.extend_from_slice(data);+ if k % 2 == 0 {+ put32(&mut out, 0x0807_4b50);+ }+ put32(&mut out, crc);+ put32(&mut out, data.len() as u32);+ put32(&mut out, data.len() as u32);+ put32(&mut central, 0x0201_4b50);+ put16(&mut central, 20);+ put16(&mut central, 20);+ put16(&mut central, 0x0808);+ put16(&mut central, 0);+ put16(&mut central, 0);+ put16(&mut central, 0x21);+ put32(&mut central, crc);+ put32(&mut central, data.len() as u32);+ put32(&mut central, data.len() as u32);+ put16(&mut central, n.len() as u16);+ put16(&mut central, 0);+ put16(&mut central, 0);+ put16(&mut central, 0);+ put16(&mut central, 0);+ put32(&mut central, 0);+ put32(&mut central, offset);+ central.extend_from_slice(n);+ }+ let cd_offset = out.len() as u32;+ let cd_len = central.len() as u32;+ out.extend_from_slice(¢ral);+ put32(&mut out, 0x0605_4b50);+ put16(&mut out, 0);+ put16(&mut out, 0);+ put16(&mut out, entries.len() as u16);+ put16(&mut out, entries.len() as u16);+ put32(&mut out, cd_len);+ put32(&mut out, cd_offset);+ put16(&mut out, 0);+ out+ }++ #[test]+ fn stored_entries_with_data_descriptors_keep_their_data() {+ let z = streamed(&[("a.json", b"{}"), ("b.txt", b"hello"), ("c.txt", b"xyz")]);+ let back = super::read_stored(&z);+ assert_eq!(back.len(), 3);+ assert_eq!(back[0], ("a.json".to_string(), b"{}".to_vec()));+ assert_eq!(back[1], ("b.txt".to_string(), b"hello".to_vec()));+ assert_eq!(back[2], ("c.txt".to_string(), b"xyz".to_vec()));+ }++ #[test]+ fn bad_central_directory_offsets_and_truncation_do_not_panic() {+ let mut z = streamed(&[("a.json", b"{}"), ("b.txt", b"hello")]);+ // Point the second central record's local-header offset past the end of the file.+ let cd = z.len() - 22;+ let cd_offset = u32::from_le_bytes([z[cd + 16], z[cd + 17], z[cd + 18], z[cd + 19]]) as usize;+ let second = cd_offset + 46 + "a.json".len();+ z[second + 42..second + 46].copy_from_slice(&u32::MAX.to_le_bytes());+ let back = super::read_stored(&z);+ assert!(back.iter().all(|(n, _)| n != "b.txt"));+ for cut in 0..z.len() {+ for (_, bytes) in super::read_stored(&z[..cut]) {+ assert!(bytes.len() <= 5);+ }+ }+ } }