Add new APIs that allow copying zip file entries between zip files

[robmv]

The copy is done directly using the raw compressed data, avoiding decompression and recompression.

In summary, the changes:

• Add a private API for ZipFile to get the raw stream that doesn't do any CRC check or decompression.
• Add the copy_file and copy_file_rename to ZipWriter that use that raw stream to copy the file.

I get nearly 95% faster Zip file modifications when I copy nearly all files, and just ignore the files I am removing from the Zip.

closes #95

[robmv]

Just pinging this pull request. Please tell me any changes required to accept this feature.

[J0sh0nat0r]

Any idea when this will be merged? Currently depending on this branch, no issues so far, great work! :)

[rylev]

@robmv unfortunately this branch needs a bunch of conflicts to be resolved. Would you be able to do that? I can then give this a review

[Plecra]

Quickly skimming through, it doesn't look like this method validates the original zip file, which could result in a malformed archive. Ideally, the library shouldn't be able to do that by default.

I don't think using CRC should impose much extra overhead. If it causes a regression, it'd be worth making a new issue to track it

[robmv]

@robmv unfortunately this branch needs a bunch of conflicts to be resolved. Would you be able to do that? I can then give this a review

I will try to update it in the following days and see about the @Plecra suggestion about the CRC. The CRC will need the file to be decompressed If I am correct and that will add some overhead, It is probably negligible because the compression is the more CPU intensive operation and that will not be needed. But if this gives me a little overhead I will try to add an option to do the CRC or not.

In my use case I really don't need the CRC check because the files are digitally signed and the signature stored on an external file, and it is checked when then ZIP file is retrieved so the CRC check is redundant and I need speed on removing things from the file, but I understand some people will want the CRC to be validated.

[Plecra]

Ah, I had been under the impression that the CRC was performed on the compressed data. I can completely understand why you wouldn't care for that, I'd just like to make the difference clear to users. Is there a convention like _unchecked for correctness? copy_unvalidated? copy_quick? copy_iffy?

[robmv]

Rebased. The only mayor difference with the previous patch is that the implementation of make_reader() was split in half, adding make_crypto_reader(). This was done in order to be able to return ZipResult for an unsupported compression method or bad password at the same time the current code does it. If I didn't split that, the Read implementation should have to hide these errors as io::Error, breaking the current behaviour.

I still doesn't do decompression and CRC checks when copying files, if anyone has suggestions for naming an API that does that and one that doesn't, I would gladly update it.

[robmv]

@rylev Still interested on doing the review?

[rylev]

@Plecra can you review this?

[Plecra]

LGTM. Thanks @robmv, just one little nitpick

[Plecra]

In time, I'd also like to expand on the documentation of this feature. A doctest would be great.

I'd hope we could avoid using two methods here too, but I'm not sure there's a good solution with the current API

[robmv]

In time, I'd also like to expand on the documentation of this feature. A doctest would be great.

@Plecra: done!, doctest added to the raw_copy* methods.

[Plecra]

@rylev I'm happy with this. Could you give the changes a once over?

[eulerdisk]

Does this new API work only for copying from an existing ZipFile? What if you have no original ZipFile to start from?
I'm doing an application where I need to spread the same data to different zip files, it would be great if I could compress it once before and then reuse the compressed data.

[robmv]

@eulerdisk true, only from an existing file. Your use case sounds interesting, but maybe an API for that will be more complex. what if the files are larger? leave the compressed version on RAM or dump to a temporary file?

With this proposed PR API you could create a temporary ZIP archive (RAM or file backed) and get the files to reuse from there.

[eulerdisk]

With this proposed PR API you could create a temporary ZIP archive (RAM or file backed) and get the files to reuse from there.

Yep, now I can do that thanks to your PR :-)

maybe an API for that will be more complex. what if the files are larger? leave the compressed version on RAM or dump to a temporary file?

My idea is that you should be able to create a ZipFile wrapping a Read object (which than can be in RAM or Disk), then add the ZipFile to the archive. Though I'm new to this library so I don't know if that make sense.

[Plecra]

@eulerdisk Interesting idea! Compression is pretty fast, but there are bound to be use cases for caching its output.

We couldn't create a simple wrapper around Read, as it wouldn't provide somewhere to keep the compressed data. Instead, you could write to a ZipWriter<io::Cursor<Vec<u8>>> to cache the results, and copy it like @robmv explained. Maybe we could make this simpler in the future...

That said, I think it's beyond the scope of this PR and would prefer some design discussion before any code changes are proposed.

[Plecra]

Sorry about this @robmv but we had to change the API of the crypto methods, and this PR now has merge conflicts. I'll probably figure out how to resolve them in time, so I'm just letting you know it could take a while

[robmv]

Updated, hopefully this one is the winner!

[Plecra]

Lovely! I'm sorry to say I'm not sure when I'll next be able to complete the review, but this PR's a top priority 😉

[Plecra]

Ace! Finally time to get this published. Thanks for your contribution @robmv