DotNetZip.Semverd icon indicating copy to clipboard operation
DotNetZip.Semverd copied to clipboard

Cannot Extract from Zip built by windows shell

Open DidjaRedo opened this issue 7 years ago • 3 comments

I have a zip file that I made using right-click in the Windows shell, which I'm trying to extract from a test library (which uses .NET Core 2.0)

When I try to ExtractAll using DotNetZip I get an error that some file already exists. Digging a bit more, the file in question is a directory the failure is because "IsDoneWithOutputToBaseDir" doesn't recognize it as such because the Windows zip file uses backslash as the path separator but DotNetLib has a hardcoded check for forward slash to detect that an entry is a file.

I think the problem is that path normalization in incorrect in IsDoneWithOutputToBaseDir, and the following change appears to fix it:

index 5c764c2..0476934 100644
--- a/src/Zip.Shared/ZipEntry.Extract.cs
+++ b/src/Zip.Shared/ZipEntry.Extract.cs
@@ -1430,10 +1430,10 @@ bool IsDoneWithOutputToBaseDir(string baseDir, out string outFileName)
                 : Path.Combine(baseDir, f);

             // workitem 10639
-            outFileName = outFileName.Replace('/', Path.DirectorySeparatorChar);
+            outFileName = outFileName.Replace(Path.DirectorySeparatorChar, '/');

             // check if it is a directory
-            if (IsDirectory || FileName.EndsWith("/"))
+            if (IsDirectory || outFileName.EndsWith("/"))
             {
                 if (!Directory.Exists(outFileName))
                 {

Happy to push a PR but don't have permission.

DidjaRedo avatar Oct 15 '18 00:10 DidjaRedo

Anyone may push that PR; you can also edit it online on github and have github prep the pr.

haf avatar Nov 13 '18 07:11 haf

That patch is definitely incorrect; the goal is to have outFileName contain a native pathname (with backslashes on Windows) and the original code was correct in that regard.

A better fix would probably be one of:

if (IsDirectory || outFileName.EndsWith(Path.DirectorySeparatorChar))

or:

if (IsDirectory || FileName.EndsWith("/") || FileName.EndsWith(Path.DirectorySeparatorChar))

(both without the other line changed at all)

I'm happy to push that as a PR if you want me to, but my own code doesn't use Extract so it would be untested.

uecasm avatar Sep 27 '19 06:09 uecasm

You need to add unit tests to your PRs. Sorry.

haf avatar Sep 27 '19 10:09 haf