Skip to content

Commit

Permalink
feat(zip): better string encoding handling (#592)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
piksel authored Oct 10, 2021
1 parent e1e1a91 commit 612969e
Show file tree
Hide file tree
Showing 13 changed files with 319 additions and 286 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System;
using System.IO;
using System.Security.Cryptography;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

Expand Down Expand Up @@ -203,6 +204,9 @@ public bool CanPatchEntries
/// </summary>
protected byte[] AESAuthCode;

/// <inheritdoc cref="StringCodec.ZipCryptoEncoding"/>
public Encoding ZipCryptoEncoding { get; set; } = StringCodec.DefaultZipCryptoEncoding;

/// <summary>
/// Encrypt a block of data
/// </summary>
Expand Down
30 changes: 29 additions & 1 deletion src/ICSharpCode.SharpZipLib/Zip/FastZip.cs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,29 @@ public Deflater.CompressionLevel CompressionLevel
set { compressionLevel_ = value; }
}

/// <summary>
/// Reflects the opposite of the internal <see cref="StringCodec.ForceZipLegacyEncoding"/>, setting it to <c>false</c> overrides the encoding used for reading and writing zip entries
/// </summary>
public bool UseUnicode
{
get => !_stringCodec.ForceZipLegacyEncoding;
set => _stringCodec.ForceZipLegacyEncoding = !value;
}

/// <summary> Gets or sets the code page used for reading/writing zip file entries when unicode is disabled </summary>
public int LegacyCodePage
{
get => _stringCodec.CodePage;
set => _stringCodec.CodePage = value;
}

/// <inheritdoc cref="StringCodec"/>
public StringCodec StringCodec
{
get => _stringCodec;
set => _stringCodec = value;
}

#endregion Properties

#region Delegates
Expand Down Expand Up @@ -456,7 +479,7 @@ private void CreateZip(Stream outputStream, string sourceDirectory, bool recurse
NameTransform = new ZipNameTransform(sourceDirectory);
sourceDirectory_ = sourceDirectory;

using (outputStream_ = new ZipOutputStream(outputStream))
using (outputStream_ = new ZipOutputStream(outputStream, _stringCodec))
{
outputStream_.SetLevel((int)CompressionLevel);
outputStream_.IsStreamOwner = !leaveOpen;
Expand Down Expand Up @@ -631,6 +654,10 @@ private void ProcessFile(object sender, ScanEventArgs e)
using (FileStream stream = File.Open(e.Name, FileMode.Open, FileAccess.Read, FileShare.Read))
{
ZipEntry entry = entryFactory_.MakeFileEntry(e.Name);
if (_stringCodec.ForceZipLegacyEncoding)
{
entry.IsUnicodeText = false;
}

// Set up AES encryption for the entry if required.
ConfigureEntryEncryption(entry);
Expand Down Expand Up @@ -967,6 +994,7 @@ private static bool NameIsValid(string name)
private INameTransform extractNameTransform_;
private UseZip64 useZip64_ = UseZip64.Dynamic;
private CompressionLevel compressionLevel_ = CompressionLevel.DEFAULT_COMPRESSION;
private StringCodec _stringCodec = new StringCodec();

private string password_;

Expand Down
43 changes: 0 additions & 43 deletions src/ICSharpCode.SharpZipLib/Zip/ZipConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -471,48 +471,5 @@ public static class ZipConstants
public const int ENDSIG = 'P' | ('K' << 8) | (5 << 16) | (6 << 24);

#endregion Header Signatures

/// <summary>
/// Default encoding used for string conversion. 0 gives the default system OEM code page.
/// Using the default code page isnt the full solution necessarily
/// there are many variable factors, codepage 850 is often a good choice for
/// European users, however be careful about compatability.
/// </summary>
[Obsolete("Use ZipStrings instead")]
public static int DefaultCodePage
{
get => ZipStrings.CodePage;
set => ZipStrings.CodePage = value;
}

/// <summary> Deprecated wrapper for <see cref="ZipStrings.ConvertToString(byte[], int)"/></summary>
[Obsolete("Use ZipStrings.ConvertToString instead")]
public static string ConvertToString(byte[] data, int count)
=> ZipStrings.ConvertToString(data, count);

/// <summary> Deprecated wrapper for <see cref="ZipStrings.ConvertToString(byte[])"/></summary>
[Obsolete("Use ZipStrings.ConvertToString instead")]
public static string ConvertToString(byte[] data)
=> ZipStrings.ConvertToString(data);

/// <summary> Deprecated wrapper for <see cref="ZipStrings.ConvertToStringExt(int, byte[], int)"/></summary>
[Obsolete("Use ZipStrings.ConvertToStringExt instead")]
public static string ConvertToStringExt(int flags, byte[] data, int count)
=> ZipStrings.ConvertToStringExt(flags, data, count);

/// <summary> Deprecated wrapper for <see cref="ZipStrings.ConvertToStringExt(int, byte[])"/></summary>
[Obsolete("Use ZipStrings.ConvertToStringExt instead")]
public static string ConvertToStringExt(int flags, byte[] data)
=> ZipStrings.ConvertToStringExt(flags, data);

/// <summary> Deprecated wrapper for <see cref="ZipStrings.ConvertToArray(string)"/></summary>
[Obsolete("Use ZipStrings.ConvertToArray instead")]
public static byte[] ConvertToArray(string str)
=> ZipStrings.ConvertToArray(str);

/// <summary> Deprecated wrapper for <see cref="ZipStrings.ConvertToArray(int, string)"/></summary>
[Obsolete("Use ZipStrings.ConvertToArray instead")]
public static byte[] ConvertToArray(int flags, string str)
=> ZipStrings.ConvertToArray(flags, str);
}
}
10 changes: 6 additions & 4 deletions src/ICSharpCode.SharpZipLib/Zip/ZipEntry.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.IO;
using System.Text;

namespace ICSharpCode.SharpZipLib.Zip
{
Expand Down Expand Up @@ -150,7 +151,7 @@ private enum Known : byte
/// The name passed is null
/// </exception>
public ZipEntry(string name)
: this(name, 0, ZipConstants.VersionMadeBy, CompressionMethod.Deflated)
: this(name, 0, ZipConstants.VersionMadeBy, CompressionMethod.Deflated, true)
{
}

Expand All @@ -171,7 +172,7 @@ public ZipEntry(string name)
/// </exception>
internal ZipEntry(string name, int versionRequiredToExtract)
: this(name, versionRequiredToExtract, ZipConstants.VersionMadeBy,
CompressionMethod.Deflated)
CompressionMethod.Deflated, true)
{
}

Expand All @@ -182,6 +183,7 @@ internal ZipEntry(string name, int versionRequiredToExtract)
/// <param name="madeByInfo">Version and HostSystem Information</param>
/// <param name="versionRequiredToExtract">Minimum required zip feature version required to extract this entry</param>
/// <param name="method">Compression method for this entry.</param>
/// <param name="unicode">Whether the entry uses unicode for name and comment</param>
/// <exception cref="ArgumentNullException">
/// The name passed is null
/// </exception>
Expand All @@ -193,7 +195,7 @@ internal ZipEntry(string name, int versionRequiredToExtract)
/// It is not generally useful, use the constructor specifying the name only.
/// </remarks>
internal ZipEntry(string name, int versionRequiredToExtract, int madeByInfo,
CompressionMethod method)
CompressionMethod method, bool unicode)
{
if (name == null)
{
Expand All @@ -216,7 +218,7 @@ internal ZipEntry(string name, int versionRequiredToExtract, int madeByInfo,
this.versionToExtract = (ushort)versionRequiredToExtract;
this.method = method;

IsUnicodeText = ZipStrings.UseUnicode;
IsUnicodeText = unicode;
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions src/ICSharpCode.SharpZipLib/Zip/ZipEntryFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public enum TimeSetting
public ZipEntryFactory()
{
nameTransform_ = new ZipNameTransform();
isUnicodeText_ = ZipStrings.UseUnicode;
isUnicodeText_ = true;
}

/// <summary>
Expand Down Expand Up @@ -162,7 +162,7 @@ public int SetAttributes
}

/// <summary>
/// Get set a value indicating whether unidoce text should be set on.
/// Get set a value indicating whether unicode text should be set on.
/// </summary>
public bool IsUnicodeText
{
Expand Down
Loading

0 comments on commit 612969e

Please sign in to comment.