Re: [PATCH v2] archive-zip: Add zip64 headers when file size is too large for 32 bits

[Peter Krefting <peter@softwolves.pp.se>]

Johannes Sixt:

> Let's get the naming straight: There is no "zip64 local header". There is a 
> "zip64 extra record" for the "zip local header".

Indeed, sorry for the confusion. That's what I get for trying to write 
coherent email at half past midnight :-)

> The zip64 extra data record has an offset field, but since the local 
> header does not have an offset field, the offset field in the 
> corresponding zip64 extra data record is always omitted.

Ah, now I understand, I was a bit confused, as the same code generates 
the central directory entry as the local entry.

>> @@ -376,7 +397,7 @@ static int write_zip_entry(struct archiver_args *args,
>>  	copy_le16(dirent.comment_length, 0);
>>  	copy_le16(dirent.disk, 0);
>>  	copy_le32(dirent.attr2, attr2);
>> -	copy_le32(dirent.offset, zip_offset);
>> +	copy_le32(dirent.offset, clamp_max(zip_offset, 0xFFFFFFFFU, 
>> &clamped));
>
> I don't see any provisions to write the zip64 extra header in the central 
> directory when this offset is clamped. This means that ZIP archives whose 
> size exceed 4GB are still unsupported.

The clamped flag will trigger the inclusion of the zip64 central 
directory, which contains the 64-bit offset. Should the central 
directory entry also have the zip64 extra field?

> These are wrong, I think. Entries that did not overflow must be omitted 
> entirely from the zip64 extra record, not filled with 0. This implies that 
> the payload size (.extra_size) is dynamic.

That is what I was trying to figure out, APPNOTE is extremely vague on 
the subject, but thinking back I recall that you are correct.

-- 
\\// Peter - http://www.softwolves.pp.se/