Skip to content
This repository was archived by the owner on May 15, 2024. It is now read-only.

Make it possible to add a ZipEntry from another zip file #210

Merged
merged 2 commits into from
Oct 19, 2020

Conversation

romerod
Copy link
Contributor

@romerod romerod commented Oct 19, 2020

This is the PR propessed in the #209.

Usage is like this:

using (var zip1 = ZipFile.Read(stream1))
using (var zip2 = ZipFile.Read(stream2))
{
    foreach (var entry in zip2.ToArray())
    {
        var newEntry = entry.CloneForNewZipFile(zip1);        
        newEntry.FileName = $@"zip2/{entry.FileName}";
        zipMain.AddEntry(newEntry);
    }
    zip1.Save(@"C:\temp\merged.zip");
}

throw new InvalidOperationException("The entry you are trying to add is a directory.");
}

if (!this.CompressionMethod.Equals(newZipFile.CompressionMethod))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a check like this one for _Encryption and _Password?

Copy link
Contributor Author

@romerod romerod Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the check on Encryption and Password, but had to remove them as they are not set on the zip1.

As I checked encryption can be mixed and as I don't decrypt I don't need the password.

Thanks for the fast review!

Copy link
Collaborator

@darkms darkms Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I checked encryption can be mixed and as I don't decrypt I don't need the password.

Ah, fair enough, just checked an it is indeed possible to have both encrypted and non-encrypted entries in the same zip, I guess it might be possible to have different passwords for them too.

@darkms
Copy link
Collaborator

darkms commented Oct 19, 2020

Awesome job!
Just left a small comment about encryption & password which I think would be important to validate in order to avoid security bugs and malformed archives. Happy to merge afterwards.

@darkms darkms merged commit 68f412a into haf:master Oct 19, 2020
@darkms darkms mentioned this pull request Nov 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants