Port krita to use Quazip instead of KArchive, to support Zip64
AbandonedPublic

Authored by rempt on Jan 20 2019, 3:20 PM.

Details

Reviewers
None
Group Reviewers
Krita
Summary

This makes it possible to save and load .kra files that are larger than 4GiB -- depending on available memory, of course. There is an option in the settings dialog to enable this; it's off by default because older versions of Krita cannot read Zip64 files.

quazip is available on all modern linux distributions and I've added a 3rdparty project for this new dependency.

Test Plan

Create a file with zip64 on and one with zip64 off; try to load these files in Krita 4.1. The one where zip64 is on will fail to load, the one where it's off will load.

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
rempt created this revision.Jan 20 2019, 3:20 PM
Restricted Application added a project: Krita. · View Herald TranscriptJan 20 2019, 3:20 PM
rempt requested review of this revision.Jan 20 2019, 3:20 PM
rempt edited the summary of this revision. (Show Details)
dkazakov added inline comments.
CMakeLists.txt
710

Please make it an optional dependency. Otherwise it'll cause us waste a lot of time compiling it on systems where we don't need it.

A few things:

  • What is the current behaviour if the file to be saved can't fit in a typical non-zip64 zip file? (Just curious.)
  • Can it be made to save as Zip64 only when the file doesn't fit in a typical zip file?
  • What happens if it attempts to save a file larger than that the filesystem can handle (e.g. >4GB on FAT32)? I'm guessing the same as in lack of disk space, but I would hope for a more helpful error message.

And reminder for myself: Check whether the Windows shell extension can handle Zip64 (it is using libzip and I think Zip64 might be opt-in)...

rempt added a comment.Jan 22 2019, 8:36 AM
  • What is the current behaviour if the file to be saved can't fit in a typical non-zip64 zip file? (Just curious.)

Krita will write a zip file that it cannot read anymore.

  • Can it be made to save as Zip64 only when the file doesn't fit in a typical zip file?

Not easily: we cannot predict exactly when a file will become bigger than 4 GiB from the size of the image in memory.

  • What happens if it attempts to save a file larger than that the filesystem can handle (e.g. >4GB on FAT32)? I'm guessing the same as in lack of disk space, but I would hope for a more helpful error message.

I have no idea: that is not dependent on the zip library we use, but an issue, I guess, with Qt's file io.

And reminder for myself: Check whether the Windows shell extension can handle Zip64 (it is using libzip and I think Zip64 might be opt-in)...

That's a good point. https://en.wikipedia.org/wiki/Zip_(file_format)#ZIP64 says explorer can handle it...

CMakeLists.txt
710

No, it isn't optional: it replaces the karchive dependency completely, so everything that reads or writes zip files uses quazip now. I do not want us to use two zip implementations concurrently.

Ah, if it is going to the the only ZIP provider, then it should be fine. Though we should check its Unicode support. I remember we had some troubles with people saving .kra images with Russian image names in it.

rempt added a comment.EditedJan 23 2019, 8:28 AM

Yes, we explicitly set the coded to utf-8 and I've tested saving a layer with a cyrillic name, with a cyrillic document name and a cyrillic filename.

rempt abandoned this revision.Jan 23 2019, 8:57 AM
  • Can it be made to save as Zip64 only when the file doesn't fit in a typical zip file?

Not easily: we cannot predict exactly when a file will become bigger than 4 GiB from the size of the image in memory.

I'm not entirely convinced since I can find references to certain zip libraries being able to switch to Zip64 on the fly as needed (e.g. https://github.com/WeTransfer/zip_tricks/blob/master/IMPLEMENTATION_DETAILS.md). I also skimmed through small parts of minizip (the master branch though) and found code that suggest it might have the ability to do the same. I'm not familiar with the ZIP format though (all I know is the file list being at the end of file).

But if we just pretend that's not possible for now, can the code be changed to do two tries - once with Zip64 disabled, then retry with Zip64 enabled if failed?

And reminder for myself: Check whether the Windows shell extension can handle Zip64 (it is using libzip and I think Zip64 might be opt-in)...

That's a good point. https://en.wikipedia.org/wiki/Zip_(file_format)#ZIP64 says explorer can handle it...

Windows Explorer is not relevant since libzip has its own zip handling.

Anyway, I just tried adding a huge file (>4GB) with 7-Zip into a .kra file and the thumbnail is displayed correctly, so it's at least not broken. Though there are places in my code that truncate integers to 32-bit, they contain checks to silently error out and shouldn't cause any buffer overflow.

rempt added a comment.Jan 23 2019, 3:42 PM
  • Can it be made to save as Zip64 only when the file doesn't fit in a typical zip file?

Not easily: we cannot predict exactly when a file will become bigger than 4 GiB from the size of the image in memory.

I'm not entirely convinced since I can find references to certain zip libraries being able to switch to Zip64 on the fly as needed (e.g. https://github.com/WeTransfer/zip_tricks/blob/master/IMPLEMENTATION_DETAILS.md). I also skimmed through small parts of minizip (the master branch though) and found code that suggest it might have the ability to do the same. I'm not familiar with the ZIP format though (all I know is the file list being at the end of file).

But if we just pretend that's not possible for now, can the code be changed to do two tries - once with Zip64 disabled, then retry with Zip64 enabled if failed?

I wouldn't want to do that, because in the case where it would be needed, it would nearly double the saving time, and for such huge files, that's already really long.