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

[René Scharfe <l.s.r@xxxxxx>]

Am 22.04.2017 um 23:52 schrieb Johannes Sixt:
> 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.

The offset is declared as unsigned int, so will wrap on most platforms
before reaching the clamp check.  At least InfoZIP's unzip can handle
that, but it's untidy.

The offset is only needed in the ZIP64 extra record for the central
header (in zip_dir) -- the local header has no offset field.  That said,
I haven't been able to implement proper 64 bit offset support so far.

René