Skip to content

feat(zip): better string encoding handling #592

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

Merged
merged 7 commits into from
Oct 10, 2021
Merged

Conversation

piksel
Copy link
Member

@piksel piksel commented Mar 7, 2021

This replaces the global static ZipStrings singleton with instances of StringCodec, which will:

  • Remove encoding configuration from a shared global state
  • Allow for different defaults for input and output
  • Explicitly override the encodings used for ZipCrypto and zip archive comments (the one in the Central Directory, not the individual entry comments).
  • Use "Unicode" for new entries (unless overriden)
  • Make it much more clear (hopefully) how and why different encodings are used.

Since the only way to customize Encoding handling in zipfiles until now has been to modify the static ZipStrings singleton, and we don't want to break backwards compatibility, it will now use separate instances for all ZipFile/Zip*Stream instances unless ZipStrings has been modified.
It's still not a great API, but the properties are marked as deprecated and should be removable in a later release.

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.

@piksel

This comment has been minimized.

@Numpsy

This comment has been minimized.

@piksel

This comment has been minimized.

@Numpsy

This comment has been minimized.

@piksel piksel force-pushed the zipstrings-instance branch from 99c232a to 3b126e9 Compare March 7, 2021 22:06
@piksel

This comment has been minimized.

@codecov
Copy link

codecov bot commented Oct 9, 2021

Codecov Report

Merging #592 (9c5f233) into master (e1e1a91) will decrease coverage by 0.02%.
The diff coverage is 70.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #592      +/-   ##
==========================================
- Coverage   73.82%   73.79%   -0.03%     
==========================================
  Files          69       68       -1     
  Lines        8291     8332      +41     
==========================================
+ Hits         6121     6149      +28     
- Misses       2170     2183      +13     
Impacted Files Coverage Δ
src/ICSharpCode.SharpZipLib/Zip/ZipStrings.cs 46.51% <47.36%> (-22.72%) ⬇️
src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs 77.28% <76.47%> (-0.05%) ⬇️
src/ICSharpCode.SharpZipLib/Zip/FastZip.cs 59.16% <80.00%> (+0.59%) ⬆️
...ib/Zip/Compression/Streams/DeflaterOutputStream.cs 74.76% <100.00%> (+0.23%) ⬆️
src/ICSharpCode.SharpZipLib/Zip/ZipEntry.cs 90.90% <100.00%> (ø)
src/ICSharpCode.SharpZipLib/Zip/ZipEntryFactory.cs 83.50% <100.00%> (ø)
src/ICSharpCode.SharpZipLib/Zip/ZipFormat.cs 89.54% <100.00%> (ø)
src/ICSharpCode.SharpZipLib/Zip/ZipInputStream.cs 80.39% <100.00%> (+0.29%) ⬆️
src/ICSharpCode.SharpZipLib/Zip/ZipOutputStream.cs 87.95% <100.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1e1a91...9c5f233. Read the comment docs.

@piksel piksel marked this pull request as ready for review October 9, 2021 21:29
@piksel piksel changed the title Replace static ZipStrings with instances of StringCodec feat(zip): better string encoding handling Oct 10, 2021
@piksel piksel merged commit 612969e into master Oct 10, 2021
@piksel piksel deleted the zipstrings-instance branch October 10, 2021 00:21
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.

Can't use Password with Unicode ZipStrings is static. Can't use different encodings for different zip files
2 participants