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

Don't call CleanName from the ZipEntry constructor. #362

Merged
merged 2 commits into from
Jan 27, 2020

Conversation

Numpsy
Copy link
Contributor

@Numpsy Numpsy commented Jun 22, 2019

Possible fix for #341 - don't call CleanName in the ZipEntry constructor.

This matches the doc comments on one of the ZipEntry constructors which says that the name format isn't enforced anyway. Not sure if this causes any gaps where names won't be validated on creating zips - not sure whether it'd be better to do some validation for 'new' ZipEntries, but just not for ones created internally when reading an existing zip?

Also marked as WIP because on further testing it seems that a lot of the validation for the characters in paths has been removed from .NET Core, meaning that the exception described in #341 might only happen in the full version of .NET (and as the unit tests are only built as Core, the exception won't be seen there).

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.

@hoborm
Copy link

hoborm commented Jan 22, 2020

Hi!
Do you have any roadmap for the next release including this MR or do you suggest to build and package it to a NuGet package if I'd like to use it? I don't want to hurry the release, I just would like to know the plans.

@Numpsy
Copy link
Contributor Author

Numpsy commented Jan 24, 2020

I don't know what the plans for the next release are, but you could maybe look at the CI builds if you need a build of it?

@hoborm
Copy link

hoborm commented Jan 27, 2020

I don't know what the plans for the next release are, but you could maybe look at the CI builds if you need a build of it?

Oh I didn't notice that it's already available in pr. Thank you once again! :)

Copy link
Member

@piksel piksel left a comment

Choose a reason for hiding this comment

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

I have yet to find the reason for this change (that this PR effectively reverts). If we need it, it should at least be a opt in/out. Better to just remove it for now.

@piksel piksel merged commit 7fd6d65 into icsharpcode:master Jan 27, 2020
@Numpsy Numpsy deleted the rw/341 branch January 28, 2020 10:02
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

Successfully merging this pull request may close these issues.

3 participants