Impossible to create .zip files with version_needed < 45

It seems to be impossible to create a .zip file that contains a version_needed field smaller than 45 (meaning version 4.5). This in turn means that minizip 2.x cannot be used to create valid Office Open XML (e.g. .xlsx) or OpenDocument (e.g. .ods) files, because — due to the minimum required unzip version of 4.5 indicated by the headers —, MS Office and LibreOffice refuse to open such files and/or need to "fix" them before opening. These apps require version_needed to be 30 or lower (at the same time they don't mind the ZIP64 extensions, if the sizes/offsets stay below 32-bit.)

The cause of the issue is that the version_needed value calculated for the central entry is forced to be no less than the pre-calculated value (which is always 45 or higher). There are cases when a .zip file created with version 4.5 features (= ZIP64 extensions) can be safely unzipped using a 2.0 compliant unzip, e.g. when the actual file sizes/offsets didn't break the 32-bit limits.

This patch fixes it:

diff --git a/mz_zip.c b/mz_zip.c
index 7c19400..5cd9f06 100644
--- a/mz_zip.c
+++ b/mz_zip.c
@@ -614,8 +614,6 @@ static int16_t mz_zip_entry_get_version_needed(int16_t zip64, mz_zip_file *file_
 
     if (file_info == NULL)
         return MZ_PARAM_ERROR;
-    if (file_info->version_needed > 0)
-        version_needed = file_info->version_needed;
     if (zip64)
         version_needed = 45;
 #ifdef HAVE_AES

Allow version_needed to be less than customized. #258

Take a look at the commit I made. It should fix the issue.

Thanks! I'm afraid the patch makes it fall into a different problem by omitting the version_needed detection on file close, making it retain its initial (and incorrect) value (45) which got determined on file add without knowing all the details (such as uncompressed file size.)

[ The interesting thing with all this is that the correct/expected behaviour will result in a different version_needed value in the local entry header vs. the central entry one (the latter being the accurate one and the one actually read by unzip tools.) This is how the traditional minizip library works. ]

Are you manually setting file_info->versIon_needed to 20 when opening the zip entry for writing?

The traditional minizip library does a seek backwards into the local header and fills in the correct values. I don't want to do a seek backwards. So if the file_info->uncompressed_size is 0 and it is the local header, then assumes zip64 (45). You are right in that it could result in different values for the version_needed and I am okay with that if there is no better solution. It is possible to set the file_info->uncompressed_size prior to calling mz_zip_entry_open and it should fill it in in both places properly right?

I'm not setting file_info->version_needed (couldn't find a way to do this with the traditional API.) [ I'm setting version_madeby (to various test values ranging from 20 to 63) when calling zipOpenNewFileInZip4_64() in my tests, OP and minizip1 works fine with those values. ]

As for setting the uncompressed_size in file open: That could work, but would break with input streams and it'd mean an incompatibility with the original minizip1 API, which didn't require it.

minizip1 indeed rewrites the local file entry but as per the source code and local tests, but it doesn't do so for the version_needed value (only for CRC32 and size fields.)

(As per new tests, the "adventure" of making .zips compatible with Excel (2016 even) isn't over with the version_needed fix. I'm still investigating.)

This extended patch avoids the other pitfall with Excel (2016), by omitting the 4 extrafield ZIP64 bytes in the headers. It does so by honoring the zip64 parameter of the zipOpenNewFileInZip*() compatibility functions (in current code this flag is ignored and ZIP64 extrafield header gets unconditionally enabled.)
Patch retains binary compatibility of mz_zip*() API.

diff --git a/mz_compat.c b/mz_compat.c
index 08a97fd..95a9dfb 100644
--- a/mz_compat.c
+++ b/mz_compat.c
@@ -146,6 +146,10 @@ extern int ZEXPORT zipOpenNewFileInZip5(zipFile file, const char *filename, cons
     file_info.version_madeby = version_madeby;
     file_info.comment = (char *)comment;
     file_info.flag = flag_base;
+    if (zip64)
+        file_info.zip64_allowed = 1;
+    else
+        file_info.zip64_allowed = 2;
 #ifdef HAVE_AES
     if (password)
         file_info.aes_version = MZ_AES_VERSION;
diff --git a/mz_zip.c b/mz_zip.c
index 41e69ba..30e11ce 100644
--- a/mz_zip.c
+++ b/mz_zip.c
@@ -867,8 +867,11 @@ static int32_t mz_zip_entry_write_header(void *stream, uint8_t local, mz_zip_fil
     if (file_info->disk_offset >= UINT32_MAX)
         extrafield_zip64_size += 8;
 
+    if ((file_info->zip64_allowed == 2) && (extrafield_zip64_size > 0))
+        return MZ_PARAM_ERROR;
+
     // Use 64-bit data descriptor if we don't know uncompressed size ahead of time
-    zip64 = (local && file_info->uncompressed_size == 0) || (extrafield_zip64_size > 0);
+    zip64 = (file_info->zip64_allowed != 2) && ((local && file_info->uncompressed_size == 0) || (extrafield_zip64_size > 0));
 
     if (zip64)
     {
@@ -889,8 +892,7 @@ static int32_t mz_zip_entry_write_header(void *stream, uint8_t local, mz_zip_fil
         extrafield_size += extrafield_ntfs_size;
     }
 
-    if (file_info->version_needed == 0)
-        file_info->version_needed = mz_zip_entry_get_version_needed(zip64, file_info);
+    file_info->version_needed = mz_zip_entry_get_version_needed(zip64, file_info);
 
     if (local)
         err = mz_stream_write_uint32(stream, MZ_ZIP_MAGIC_LOCALHEADER);
diff --git a/mz_zip.h b/mz_zip.h
index 2033836..7fdf9dc 100644
--- a/mz_zip.h
+++ b/mz_zip.h
@@ -47,6 +47,7 @@ typedef struct mz_zip_file_s
     uint64_t disk_offset;               // relative offset of local header
     uint16_t internal_fa;               // internal file attributes
     uint32_t external_fa;               // external file attributes
+    uint16_t zip64_allowed;             // ZIP64 allowed (0=auto, 1=yes, 2=no)
 
     char     *filename;                 // filename string
     uint8_t  *extrafield;               // extrafield data

Are you still investigating or does this fix all the compatibility issues with Excel?

I'm finished (for the context of this patch at least). Above two fixes cover the issues I/we experienced with Office 2016, 2007 (probably) and LibreOffice.

Allow configuration of zip64 support. #258

Thank you, I've seen a56a08c. I haven't tested it yet, but it LGTM.

The initial issue remains however, where it's still not possible to create .zip with version_needed < 45. This causes corruption errors/warnings with both LibreOffice and MS Offices (or any other app that's not prepared to handle .zip versions above 2.0.) For that to be fixed, this hunk of the above patch would need to be used:

diff --git a/mz_compat.c b/mz_compat.c
index 08a97fd..95a9dfb 100644
--- a/mz_compat.c
+++ b/mz_compat.c
@@ -889,8 +892,7 @@ static int32_t mz_zip_entry_write_header(void *stream, uint8_t local, mz_zip_fil
         extrafield_size += extrafield_ntfs_size;
     }
 
-    if (file_info->version_needed == 0)
-        file_info->version_needed = mz_zip_entry_get_version_needed(zip64, file_info);
+    file_info->version_needed = mz_zip_entry_get_version_needed(zip64, file_info);
 
     if (local)
         err = mz_stream_write_uint32(stream, MZ_ZIP_MAGIC_LOCALHEADER);

UPDATE: I managed to test and it turns out, that the zip64 fix resolved the second issue with the above extra patch. Great stuff! I'm going to integrate everything and retest once more.

I don't understand how that is preventing it from being created with version_needed < 45. If I use MZ_ZIP64_DISABLE then it works. I do the if statement to allow for a custom value to be used.

You're right, I haven't considered the effect of the second (zip64) fix to the initial issue. After fixing the zip64 one, the version_needed value initially calculated will be 20, so there is not need to revise that when writing the central entry. So it solves the non-zip64 case perfectly.

[ For the rest of similar cases (should they exist), one should zero the field to have it recalculated again, which is fine when using the native mz_*() API. ]

All in all, I'm perfectly happy with this. Thank you very much for following through this and giving these patches their proper final form!

Simplification & fixed formatting. #258

Add back auto detection of zip64. #258

Navigation Menu

Repository navigation

Impossible to create .zip files with `version_needed` < 45 #258

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Impossible to create .zip files with version_needed < 45 #258

Description

Activity

nmoinvaz commented on May 2, 2018

vszakats commented on May 2, 2018

nmoinvaz commented on May 2, 2018

nmoinvaz commented on May 2, 2018

vszakats commented on May 2, 2018

vszakats commented on May 2, 2018

nmoinvaz commented on May 3, 2018

vszakats commented on May 3, 2018

vszakats commented on May 3, 2018

nmoinvaz commented on May 3, 2018

vszakats commented on May 3, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Impossible to create .zip files with `version_needed` < 45 #258