Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ZIP64 creation with individual files larger 4GB and new archive sample #730

Closed
wants to merge 3 commits into from

Conversation

TcT2k
Copy link
Contributor

@TcT2k TcT2k commented Feb 14, 2018

If single files larger than 4GB where added the recorded file sizes
in the local file header where not updated correctly. Additionally
recorded file sizes in the central directory where in the wrong order
making it impossible to extract.

To enable adding files with unknown size which might be larger than 4GB
the new method wxZipOutputStream::SetFormat() is added.

Additionally a check has been added in case a file larger 4GB has been
written without ZIP64 info.

I've also created a new sample archive to show using wxArchiveStreams

Additionally potential issue when reading ZIP64 files is fixed.

@MaartenBent
Copy link
Contributor

The archive sample fails to build (with makefile.gcc) because it tries to link with core instead of base.

@TcT2k
Copy link
Contributor Author

TcT2k commented Feb 14, 2018

@MaartenBent thanks for testing, I've corrected the linked libs

@MaartenBent
Copy link
Contributor

There are still some link errors, but a lot less. I think you need to use

template="wx_sample_console" template_append="wx_append_base"

like the console and secretstore samples.

(secretstore does not use wx_append_base, but all other console samples do. Is this an errror @vadz ?)

@TcT2k
Copy link
Contributor Author

TcT2k commented Feb 15, 2018

Changed the bkl to use wx_sample_console template

@TcT2k TcT2k force-pushed the sample_archive branch 3 times, most recently from dbf6320 to 946fff0 Compare February 15, 2018 10:58
@MaartenBent
Copy link
Contributor

Yes, now it works 👍.
It has the following build warning:

g++ -c -o gcc_mswu64\archive_archive.o  -O2 -mthreads  -DHAVE_W32API_H -D__WXMSW__   -DNDEBUG    -D_UNICODE -I.\..\..\lib\gcc_lib64\mswu -I.\..\..\include  -W -Wall -I.  -DwxUSE_GUI=0   -Wno-ctor-dtor-privacy -m64 -std=c++11 -MTgcc_mswu64\archive_archive.o -MFgcc_mswu64\archive_archive.o.d -MD -MP archive.cpp
archive.cpp:77:1: warning: missing initializer for member 'ArchiveApp::ArchiveCommandDesc::id' [-Wmissing-field-initializers]
 };
 ^
archive.cpp:77:1: warning: missing initializer for member 'ArchiveApp::ArchiveCommandDesc::desc' [-Wmissing-field-initializers]

Creating a zip with a 6gb file using the sample correctly enables zip64 and 7zip does not complain.

When I use --force-zip64 option and create a zip file of a small file (110KB), zip64 is not enabled. 7zip shows the same archive info with or without the option (both version 20).
It seems z64Required results in false in wxZipEntry::WriteCentral, and wxZipEndRec::Write also does not go into zip64 specific code. Is this intended?

Specific values where not correctly converted because of
a signed/unsigned mismatch.
If single files larger than 4GB where added the recorded file sizes
in the local file header where not updated correctly. Additionally
recorded file sizes in the central directory where in the wrong order
making it impossible to extract.

To enable adding files with unknown size which might be larger than 4GB
the new method wxZipOutputStream::SetFormat() is added.

Additionally a check has been added in case a file larger 4GB has been
written without ZIP64 info.
This sample shows usage of wxArchiveStream and wxArchiveFactory.
It also allows for easy testing of wxArchiveStream implementations
outside of the unit tests.
@TcT2k
Copy link
Contributor Author

TcT2k commented Feb 15, 2018

@MaartenBent
I've fixed the warning in the sample.

Regarding the info 7-zip shows: it uses version to extract from the central directory but other infos from local. But just to be sure I've changed the version in the central directory too. Which I think shouldn't matter much, but just seems right. --force-zip64 does not change anything in the central directory but only in the local file info.

@vadz
Copy link
Contributor

vadz commented Feb 16, 2018

I'm a bit afraid of adding a new sample, which might fail to compile in some untested build configuration, just before 3.1.1, but OTOH I definitely do want to fix ZIP64 support before it. So I'd rather merge the first 2 commits right now, but leave the sample until after 3.1.1, or do you think it's absolutely and totally safe to merge the commit adding the sample right now as well?

@TcT2k
Copy link
Contributor Author

TcT2k commented Feb 16, 2018

I wouldn't expect any issues with the sample. but I think it's reasonable to postpone it to after 3.1.1 and just include the fixes just to be on the safe side.

@vadz
Copy link
Contributor

vadz commented Feb 16, 2018

Thanks, I'm merging the first 2 commits with some minor fixes to the documentation (please review them if possible to check if I misunderstood anything) and will merge the commit adding the sample after 3.1.1.

@vadz vadz added this to the 3.1.2 milestone Feb 16, 2018
vadz added a commit that referenced this pull request Feb 16, 2018
Notably fix ZIP64 creation with individual files larger than 4GB.

See #730
@TcT2k
Copy link
Contributor Author

TcT2k commented Feb 16, 2018

@vadz your documentation changes in a3673be seem good too me.

@TcT2k
Copy link
Contributor Author

TcT2k commented Feb 16, 2018

@vadz On issue in a3673be is an open comment which would probably fail with doxygen

@vadz
Copy link
Contributor

vadz commented Feb 17, 2018

Oops, thanks for noticing this, will fix (no idea how did it happen...).

vadz added a commit that referenced this pull request Feb 17, 2018
Fix an accidental removal of the closing comment marker in
a3673be.

See #730
vadz pushed a commit that referenced this pull request Feb 20, 2018
This sample shows usage of wxArchiveStream and wxArchiveFactory.
It also allows for easy testing of wxArchiveStream implementations
outside of the unit tests.

See #730
@vadz
Copy link
Contributor

vadz commented Feb 20, 2018

I've added the commit adding the sample too now, so there should be nothing left here. Thanks again!

@vadz vadz closed this Feb 20, 2018
@TcT2k TcT2k deleted the sample_archive branch February 20, 2018 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants