Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FastZip's NameTransform property is settable, but not honored #275

Open
Zastai opened this issue Oct 1, 2018 · 3 comments
Open

FastZip's NameTransform property is settable, but not honored #275

Zastai opened this issue Oct 1, 2018 · 3 comments
Assignees

Comments

@Zastai
Copy link

Zastai commented Oct 1, 2018

Steps to reproduce

  1. create a FastZip instance
  2. set its NameTransform property to an INameTransform implementation
  3. execute its CreateZip method to create a zip file

Expected behavior

The configured name transform is used for the zip file creation.

Actual behavior

The configured name provider is ignored; CreateZip always sets it to a new ZipNameTransform instance.
If this is the intent, perhaps the documentation should better reflect this, and the setter should then probably be private (or protected, given that FastZip isn't sealed).

Note: the same happens if you set the EntryFactory to a custom entry factory with a custom name transform - CreateZip happily changes the factory's name transform. That sounds like highly irregular behavior.

Version of SharpZipLib

1.0

Obtained from (place an x between the brackets for all that apply)

  • Package installed using:
    • NuGet
@piksel piksel self-assigned this Oct 13, 2018
@piksel
Copy link
Member

piksel commented Nov 12, 2018

Yes, that does seem counter-intuitive. CreateZip should use the backing field (entryFactory_.NameTransform) if it's not null or otherwise create a new a new instance the same way that it does right now. My guess is that CreateZip is meant to be used as a shortcut, when you only want the most basic behaviour. But then I guess it should either be static or clearly separated from the rest of the class.

A PR would be welcomed!

@Zastai
Copy link
Author

Zastai commented Nov 15, 2018

Will look at a making a PR when I have time. Maybe this weekend, but no promises.

@Numpsy
Copy link
Contributor

Numpsy commented Mar 29, 2020

Looking at the FastZip source, there are two name transform instances used by FastZip:

  1. The mentioned property, which gets set to a new instance of ZipNameTransform by CreateZip.
  2. A private 'extractNameTransform_' field which is set to an instance of WindowsNameTransform by ExtractZip().

Not sure what the logic is for (sort of) allowing the transform for Create to be specified, but not the one for Extract ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants