Invalid Characters in Zip throws exception

[RevenantBob]

Steps to reproduce

1. Use a Zip file produced on a *nix system or MacOS (In this case)
2. Zip files with invalid windows characters that are valid on MacOS/Linux
3. Open up Zip archive with SharpZipLib API

Exception is thrown.

Expected behavior

SharpZipLib should not load a ZipFile class and try to clean up the entry filenames before returning them to the user. The user should be responsible for cleaning up entries themselves in whatever method they see fit.

Actual behavior

SharpZipLib throws an exception in ZipEntry line 1303 because IsPathRooted checks for invalid characters. It also removes root path names.

Version of SharpZipLib

1.10

Obtained from (only keep the relevant lines)

• Package installed using NuGet

For now I worked around the issues by removing the CleanName function from ZipEntry in my own local copy.

[piksel]

Version 1.10? I'm guessing it's 1.1.0?

[Numpsy]

Confirmed with the current source using a zip file which contains a file that has question marks in the name - it throws a System.ArgumentException from Path.IsPathRooted.

Given that the comments on the CleanName function talk about converting names into zip formats, doesn't sound like something that should be called on extraction?
The ZipEntry constructor comment at

SharpZipLib/src/ICSharpCode.SharpZipLib/Zip/ZipEntry.cs

Line 163 in f03a2ef

/// path elements separated by '/' characters. This is not enforced see <see cref="CleanName(string)">CleanName</see>

also says that the name format isn't enforced, so it might be misleading that the constructor chain does then call CleanName?

[piksel]

Agreed. It should at least be platform-specific, but then again, we could just let the file system throw the exception on invalid file names instead.

[Numpsy]

There's a bunch of functionality in the NameTransform bits for processing invalid characters and such that the caller and/or container could use rather than ZipEntry doing it, including some alternate path root handling code that CleanName could possibly use instead of IsPathRooted if required?

Related: Most of the Add() functions on ZipFile create ZipEntrys using the EntryFactory, which runs the names through NameTransform, but the variants that actually take ZipEntries don't.
Would removing the CleanName calls from the ZipEntry constructor then allow you to add entries with invalid names to ZipFile, in situations where you perhaps couldn't before? (or is that just up to the caller to deal with?)

[piksel]

Yeah, now with #362 closed, we will be able to read out-of-spec files, but it also makes it possible to create them. We should at least make sure that that's not done accidentally, and use ZipNameTransform for the transforms.
Also this might effect #402 as that presumes entry file names to be transformed.

[Numpsy]

Looks like the 'WriteZipStreamWithNoCompression' unit test you added for #406 has been broken by the transform/clean change?

Something like Numpsy@4e7b20b might work to transform the names when adding entries to ZipOutputStream, not sure how the instances need to be managed though (Might ZipOutputStream need a NameTransform property similar to the one on ZipFile?)

[Numpsy]

Saying that, making zip output stream do a transform itself would also have a knock on effect in FastZip (e.g. #275) - not sure how the interactions should work there.

[Numpsy]

Hopefully resolved now?

[piksel]

Yeah!