fix: ignore a ZIP64 extra field that no sentinel asked for

[trypsynth]

The bug

Some EPUB files fail to open with Invalid local file header, while unzip, Python's zipfile and 7-Zip all read them without complaint. The archives turn out to be fine. What is wrong is one extra field, and the way this crate reads it.

Every central directory entry in the affected files carries a ZIP64 extended information extra field (id 0x0001) that has no business being there. All of the entry's sizes and its local header offset fit comfortably in 32 bits, so there is nothing for a ZIP64 block to record. Worse, the block is malformed. Here is the one on META-INF/container.xml, with the NTFS block below it for comparison:

id=0x0001 size=32  00000000 01001800 | 0e010000 00000000 | ba000000 00000000 | 3a000000 00000000
id=0x000a size=32  00000000 01001800 | <mtime> <atime> <ctime>
                   ^^^^^^^^^^^^^^^^^

A ZIP64 block is [uncompressed u64][compressed u64][header offset u64][disk u32]. This one starts with eight bytes that belong to the NTFS block's header (a reserved u32, then tag 0x0001 and size 0x0018), which the writer appears to have copied into the wrong field. Everything is shifted eight bytes right, so 0x10e (270, the real uncompressed size) is read as the uncompressed size... one slot too early, and so on down the block.

Why the crate fails on it

Zip64ExtendedInformation::parse decides field by field:

if len >= 24 || u64::from(uncompressed_size) == ZIP64_BYTES_THR { ... }

len is 32 here, so len >= 24 is true and all three values are read and applied, without ever checking whether the entry actually asked for them. After the eight byte shift the crate ends up with an uncompressed size of 0x0018010000000000, a compressed size of 270 and a header start of 186. It then seeks to 186, which is in the middle of the compressed data, finds no PK\x03\x04 there, and reports Invalid local file header.

APPNOTE 4.5.3 is explicit that a field appears in this block only when the matching field in the entry holds the 0xFFFFFFFF sentinel meaning "too large to store here, look in the ZIP64 block". None of these entries has a sentinel anywhere, so the block should never have been consulted at all. Info-ZIP unzip, Python's zipfile and 7-Zip all select by sentinel, which is exactly why they read these files. (7-Zip additionally reports a headers warning, having noticed the block is malformed, and correctly declines to act on it.)

The fix

Keep deciding what to read by the block's length, so the stream stays in step with writers that emit more fields than they strictly need to, but let a value replace the entry's own only when a sentinel asked for it.

The reading behaviour is byte for byte identical to before in every case, so nothing moves in the stream. Only which values get applied changes:

block length	entry's 32-bit field	before	after
< 24	sentinel	read and applied	unchanged
< 24	ordinary value	entry's value kept	unchanged
>= 24	sentinel	read and applied	unchanged
>= 24	ordinary value	read and applied	entry's value kept

A block that is present without a sentinel only repeats what the entry already said, so ignoring it loses nothing when the block is well formed, and it is the only way to tell a malformed block apart from a meaningful one.

One related case falls out of the same rule. A local header has no relative offset field at all, so header_start is None there, and a long block in a local header no longer yields a header_start read out of whatever followed the sizes. That already was the behaviour for a well formed local ZIP64 block, whose length is 16, so this makes the two lengths agree rather than introducing anything new.

Tests

tests/zip64_extra_field_without_sentinel.rs builds an archive with the writer and then rewrites two id bytes to attach exactly this malformed block to both the local and the central header, so no length or offset in the archive moves. Two tests cover it, one through ZipArchive and one through read_zipfile_from_stream, since the two reach the block by different paths. The first fails without the fix with InvalidArchive("Unexpected end of zip::format::blocks::ZipLocalEntryBlock"); the second passes either way and is there to keep the local header path honest.

I also checked the fix against the three real EPUBs that prompted this. All three now open and read their contents, and Python confirms every CRC in them passes, so the archives really were intact all along.

Checks

Run on Windows:

• cargo test with --no-default-features, default features and --all-features: all pass.
• cargo clippy --all-targets with all three feature sets on stable, and with --all-features on nightly: clean.
• cargo doc --no-deps with all three feature sets: only the two CompressionMethod link warnings that are already present on master.
• cargo fmt --check --all: clean.
• Builds and tests against the 1.88 MSRV.

Branch is rebased on master as of 803adde. Happy to grant write access to the branch if that would help with any later conflicts.

[Its-Just-Nans]

Hi

thanks for the report and the PR

I have a few questions

• do you have an example epub file ?
• do you know who produces such epub files ?
• what do you think about this other fix https://github.com/zip-rs/zip2/compare/other-fix-962?expand=1 which keeps the bad value in the zip64 and, as you explain, only use the zip64 when needed in apply_extra_fields()

Thanks!

[trypsynth]

Thanks for looking at this so quickly. Taking the three in turn.

An example file

There's one in the PR now, as tests/data/zip64_extra_field_without_sentinel.epub. GitHub renders it as "Binary file not shown" in the Files changed tab, so here is a direct link: zip64_extra_field_without_sentinel.epub.

It is the first two entries of one of the real EPUBs kept byte for byte (the boilerplate mimetype and META-INF/container.xml), with a fresh end of central directory record and none of the book in it. 584 bytes, and it fails on master exactly as the full file does:

entry 1: invalid Zip archive: Invalid local file header

The originals are paid ebooks so I'd rather not post one publicly, but I'm happy to send you a whole one privately if the trimmed fixture isn't enough.

Who produces them

I can describe the writer but I can't name it. The three files I have are all Scribd/Everand downloads, and the EPUB producer is not the common factor: two carry <dc:contributor opf:role="bkp">ScribdMpubToEpubConverter</dc:contributor>, the third carries calibre (4.7.0). What they share is the ZIP container, so something repackages them after the fact.

That repackager's fingerprint:

• central version made by = 0x0B17, and version needed to extract = 2.0, so it never even claims ZIP64 at the entry level
• every entry after mimetype carries the 32 byte 0x0001 block (46 of 48, 11 of 12, and 24 of 26 entries in my three files)
• the block is the correct 24 byte payload with eight bytes of NTFS block header glued to the front:

id=0x0001 size=32  00000000 01001800 | 0e010000 00000000 | ba000000 00000000 | 3a000000 00000000
id=0x000a size=32  00000000 01001800 | <mtime>           | <atime>           | <ctime>
                   ^^^^^^^^^^^^^^^^^
                   reserved u32, tag 0x0001, size 0x0018: this belongs to an NTFS block

The real values are all there and all correct (270 uncompressed, 186 compressed, offset 58); they just sit eight bytes to the right of where a reader looks. It reads like a generic "NTFS-style tagged extra field" emitter that the ZIP64 payload got run through by mistake.

Two of the three go further and are nominally ZIP64 archives: EOCD with entries = 65535 and both sizes 0xFFFFFFFF, backed by a well-formed ZIP64 EOCD + locator (version made by 4.5, 48 and 12 entries). For 490 KB and 157 KB files. So this writer reaches for ZIP64 structures whether or not anything needs them, which is the same habit that produced the bogus per-entry block.

The other fix

I like where you put it better than where I had it, and I've pushed a version that takes your placement seriously. Short version: the check belongs in apply_extra_fields, and the way to get it there is to make parse stop lying.

Both patches read identically. I built your branch and ran it against all three EPUBs and the fixture. All four open and read, and it passes the two tests already in this PR. So this isn't about which one fixes the bug; both do.

The difference is what stays in Zip64ExtendedInformation afterwards, and that type is shared with the writer. On the write side a None field already means "this entry has nothing to record here": it is what central_header() returns for a field that fits in 32 bits. But ExtraField::parse wraps everything in Some unconditionally:

ExtraField::Zip64ExtendedInformation(Zip64ExtendedInformation {
    sizes: Some(Zip64Sizes { uncompressed_size: new_uncomp, compressed_size: new_comp }),
    header_start: Some(new_head),
})

so after parsing, the struct always claims to have something to say, and the two sides disagree about what the type means. That claim leaks. ZipWriter::new_append keeps the parsed extra fields and re-emits them, and write_central_directory_header refreshes sizes but leaves header_start alone. Opening the EPUB for append and immediately finishing:

                       your branch                        this PR
META-INF/container.xml offset=0x3a  zip64=(24 bytes)      offset=0x3a  zip64=(16 bytes)
                                    [270, 186, 186]                    [270, 186]
OEBPS/content.opf      offset=0x170 zip64=(24 bytes)      offset=0x170 zip64=(16 bytes)
                                    [1763, 575, 575]                   [1763, 575]

That third value is the stale one: it is the compressed size sitting in the relative offset slot. The entry's own 32 bit offset field says 0x3a and the ZIP64 block it carries says 186, in an archive we just wrote. Your branch can fix that with one line in write_central_directory_header (else { zip64_block.header_start = None; }), but then the rule lives in two places.

So instead of filtering in apply_extra_fields, parse now returns the struct with honest Nones, and apply_extra_fields needs no change at all. Its existing if let Some(..) is the sentinel check, which I think is the shape you were reaching for. It also lets parse keep one thing it knows and apply_extra_fields doesn't: whether it's looking at a local header, which has no relative offset field to override in the first place.

The parse function got shorter rather than longer:

fn read_field<R: Read>(reader: &mut R, len: u16, consumed_len: &mut usize, is_zip64: bool)
    -> ZipResult<Option<u64>>
{
    if len < 24 && !is_zip64 {
        return Ok(None);
    }
    let value = Self::read_value(reader)?;
    *consumed_len += mem::size_of::<u64>();
    Ok(is_zip64.then_some(value))
}

Whether a value is read is still decided by the block's length, so the stream stays in step with over-eager writers; whether it is kept is decided by the sentinel.

There's a new test pinning the round trip (a_value_no_sentinel_asked_for_is_not_written_back_out). It fails on your branch and on master, and passes here.

That said, if you'd rather keep the diff in apply_extra_fields and add the one-line reset on the write side, say so and I'll switch to that. It's your call and I won't argue it further.

One thing I left alone

apply_extra_fields still does self.large_file = true for any ZIP64 block, sentinel or not. That's why appending to these EPUBs writes 0xFFFFFFFF sentinels and a ZIP64 block for a 270 byte entry at all. Gating it looks right, but large_file also feeds version_needed() and the data descriptor path, and writers that use a data descriptor put 0 rather than a sentinel in the local sizes, so a naive gate could downgrade a genuine ZIP64 streaming entry. Out of scope here, and I didn't want to guess. Happy to open a separate issue if you think it's worth chasing.

Checks

cargo test, cargo clippy --all-targets and cargo fmt --check all clean on --no-default-features, default features and --all-features. The new fixture tests avoid the deflate decoder (they go through by_index_raw) so they hold in a no-features build too.