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

[Johannes Sixt <j6t@xxxxxxxx>]

Am 22.04.2017 um 21:22 schrieb Peter Krefting:
> @@ -279,6 +299,7 @@ static int write_zip_entry(struct archiver_args *args,
>      int is_binary = -1;
>      const char *path_without_prefix = path + args->baselen;
>      unsigned int creator_version = 0;
> +    int clamped = 0;
>
>      crc = crc32(0, NULL, 0);
>
> @@ -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));
>
>      copy_le32(header.magic, 0x04034b50);
>      copy_le16(header.version, 10);
> @@ -384,15 +405,26 @@ static int write_zip_entry(struct archiver_args
> *args,
>      copy_le16(header.compression_method, method);
>      copy_le16(header.mtime, zip_time);
>      copy_le16(header.mdate, zip_date);
> -    set_zip_header_data_desc(&header, size, compressed_size, crc);
> +    set_zip_header_data_desc(&header, size, compressed_size, crc,
> &clamped);
>      copy_le16(header.filename_length, pathlen);
> -    copy_le16(header.extra_length, ZIP_EXTRA_MTIME_SIZE);
> +    copy_le16(header.extra_length, ZIP_EXTRA_MTIME_SIZE + (clamped ?
> ZIP_EXTRA_ZIP64_SIZE : 0));
>      write_or_die(1, &header, ZIP_LOCAL_HEADER_SIZE);
>      zip_offset += ZIP_LOCAL_HEADER_SIZE;
>      write_or_die(1, path, pathlen);
>      zip_offset += pathlen;
>      write_or_die(1, &extra, ZIP_EXTRA_MTIME_SIZE);
>      zip_offset += ZIP_EXTRA_MTIME_SIZE;
> +    if (clamped) {
> +        struct zip_extra_zip64 extra_zip64;
> +        copy_le16(extra_zip64.magic, 0x0001);
> +        copy_le16(extra_zip64.extra_size, ZIP_EXTRA_ZIP64_PAYLOAD_SIZE);
> +        copy_le64(extra_zip64.size, size);
> +        copy_le64(extra_zip64.compressed_size, compressed_size);
> +        copy_le64(extra_zip64.offset, zip_offset);
> +        copy_le32(extra_zip64.disk, 0);
> +        write_or_die(1, &extra_zip64, ZIP_EXTRA_ZIP64_SIZE);
> +        zip_offset += ZIP_EXTRA_ZIP64_SIZE;

Is this correct? Not all of the zip64 extra fields are always populated. 
Only those whose regular fields are filled with 0xffffffff must be 
present. Since there is only one flag, it is not possible to know which 
of the fields must be filled in.

Readers will most likely ignore trailing fields that should not be 
there; however, when the offset exceeds 32 bits, but not the compressed 
size, readers will pick the compressed size and interpret it as offset.

> +    }

-- Hannes