Merge PR #255, Correctly handle Unicode strings

* Split out string conversion from ZipConstants to ZipStrings
* Switch ZipOutputStream to using ZipStrings
* Add ZipConstants wrappers for backwards compability
* Set IsUnicodeText based upon ZipStrings default code page
* Update string transcoding to handle DefaultCodePage not Unicode
* Update tests for string coding
* Fixes #251

Author: piksel

src/ICSharpCode.SharpZipLib/Zip/ZipConstants.cs

@@ -202,7 +202,7 @@ public enum GeneralBitFlags
 	/// <summary>
 	/// This class contains constants used for Zip format files
 	/// </summary>
-	public sealed class ZipConstants
+	public static class ZipConstants
 	{
 		#region Versions
 		/// <summary>
@@ -421,166 +421,49 @@ public sealed class ZipConstants
 		public const int ENDSIG = 'P' | ('K' << 8) | (5 << 16) | (6 << 24);
 		#endregion
 
-		/// <remarks>
-		/// The original Zip specification (https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT) states 
-		/// that file names should only be encoded with IBM Code Page 437 or UTF-8. 
-		/// In practice, most zip apps use OEM or system encoding (typically cp437 on Windows). 
-		/// Let's be good citizens and default to UTF-8 http://utf8everywhere.org/
-		/// </remarks>
-		static int defaultCodePage = Encoding.UTF8.CodePage;
-
 		/// <summary>
 		/// Default encoding used for string conversion.  0 gives the default system OEM code page.
 		/// Using the default code page isnt the full solution neccessarily
 		/// there are many variable factors, codepage 850 is often a good choice for
 		/// European users, however be careful about compatability.
 		/// </summary>
-		public static int DefaultCodePage {
-			get {
-				return defaultCodePage;
-			}
-			set {
-				if ((value < 0) || (value > 65535) ||
-					(value == 1) || (value == 2) || (value == 3) || (value == 42)) {
-					throw new ArgumentOutOfRangeException(nameof(value));
-				}
-
-				defaultCodePage = value;
-			}
+		[Obsolete("Use ZipStrings instead")]
+		public static int DefaultCodePage
+		{
+			get => ZipStrings.CodePage;
+			set => ZipStrings.CodePage = value;
 		}
 
-		/// <summary>
-		/// Convert a portion of a byte array to a string.
-		/// </summary>		
-		/// <param name="data">
-		/// Data to convert to string
-		/// </param>
-		/// <param name="count">
-		/// Number of bytes to convert starting from index 0
-		/// </param>
-		/// <returns>
-		/// data[0]..data[count - 1] converted to a string
-		/// </returns>
+		/// <summary> Depracated wrapper for <see cref="ZipStrings.ConvertToString(byte[], int)"/></summary>
+		[Obsolete("Use ZipStrings.ConvertToString instead")]
 		public static string ConvertToString(byte[] data, int count)
-		{
-			if (data == null) {
-				return string.Empty;
-			}
-
-			return Encoding.GetEncoding(DefaultCodePage).GetString(data, 0, count);
-		}
+			=> ZipStrings.ConvertToString(data, count);
 
-		/// <summary>
-		/// Convert a byte array to string
-		/// </summary>
-		/// <param name="data">
-		/// Byte array to convert
-		/// </param>
-		/// <returns>
-		/// <paramref name="data">data</paramref>converted to a string
-		/// </returns>
+		/// <summary> Depracated wrapper for <see cref="ZipStrings.ConvertToString(byte[])"/></summary>
+		[Obsolete("Use ZipStrings.ConvertToString instead")]
 		public static string ConvertToString(byte[] data)
-		{
-			if (data == null) {
-				return string.Empty;
-			}
-			return ConvertToString(data, data.Length);
-		}
+			=> ZipStrings.ConvertToString(data);
 
-		/// <summary>
-		/// Convert a byte array to string
-		/// </summary>
-		/// <param name="flags">The applicable general purpose bits flags</param>
-		/// <param name="data">
-		/// Byte array to convert
-		/// </param>
-		/// <param name="count">The number of bytes to convert.</param>
-		/// <returns>
-		/// <paramref name="data">data</paramref>converted to a string
-		/// </returns>
+		/// <summary> Depracated 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)
-		{
-			if (data == null) {
-				return string.Empty;
-			}
-
-			if ((flags & (int)GeneralBitFlags.UnicodeText) != 0) {
-				return Encoding.UTF8.GetString(data, 0, count);
-			} else {
-				return ConvertToString(data, count);
-			}
-		}
+			=> ZipStrings.ConvertToStringExt(flags, data, count);
 
-		/// <summary>
-		/// Convert a byte array to string
-		/// </summary>
-		/// <param name="data">
-		/// Byte array to convert
-		/// </param>
-		/// <param name="flags">The applicable general purpose bits flags</param>
-		/// <returns>
-		/// <paramref name="data">data</paramref>converted to a string
-		/// </returns>
+		/// <summary> Depracated wrapper for <see cref="ZipStrings.ConvertToStringExt(int, byte[])"/></summary>
+		[Obsolete("Use ZipStrings.ConvertToStringExt instead")]
 		public static string ConvertToStringExt(int flags, byte[] data)
-		{
-			if (data == null) {
-				return string.Empty;
-			}
-
-			if ((flags & (int)GeneralBitFlags.UnicodeText) != 0) {
-				return Encoding.UTF8.GetString(data, 0, data.Length);
-			} else {
-				return ConvertToString(data, data.Length);
-			}
-		}
+			=> ZipStrings.ConvertToStringExt(flags, data);
 
-		/// <summary>
-		/// Convert a string to a byte array
-		/// </summary>
-		/// <param name="str">
-		/// String to convert to an array
-		/// </param>
-		/// <returns>Converted array</returns>
+		/// <summary> Depracated wrapper for <see cref="ZipStrings.ConvertToArray(string)"/></summary>
+		[Obsolete("Use ZipStrings.ConvertToArray instead")]
 		public static byte[] ConvertToArray(string str)
-		{
-			if (str == null) {
-				return new byte[0];
-			}
+			=> ZipStrings.ConvertToArray(str);
 
-			return Encoding.GetEncoding(DefaultCodePage).GetBytes(str);
-		}
-
-		/// <summary>
-		/// Convert a string to a byte array
-		/// </summary>
-		/// <param name="flags">The applicable <see cref="GeneralBitFlags">general purpose bits flags</see></param>
-		/// <param name="str">
-		/// String to convert to an array
-		/// </param>
-		/// <returns>Converted array</returns>
+		/// <summary> Depracated wrapper for <see cref="ZipStrings.ConvertToArray(int, string)"/></summary>
+		[Obsolete("Use ZipStrings.ConvertToArray instead")]
 		public static byte[] ConvertToArray(int flags, string str)
-		{
-			if (str == null) {
-				return new byte[0];
-			}
-
-			if ((flags & (int)GeneralBitFlags.UnicodeText) != 0) {
-				return Encoding.UTF8.GetBytes(str);
-			} else {
-				return ConvertToArray(str);
-			}
-		}
+			=> ZipStrings.ConvertToArray(flags, str);
 
 
-		/// <summary>
-		/// Initialise default instance of <see cref="ZipConstants">ZipConstants</see>
-		/// </summary>
-		/// <remarks>
-		/// Private to prevent instances being created.
-		/// </remarks>
-		ZipConstants()
-		{
-			// Do nothing
-		}
 	}
 }

src/ICSharpCode.SharpZipLib/Zip/ZipEntry.cs

@@ -191,6 +191,8 @@ internal ZipEntry(string name, int versionRequiredToExtract, int madeByInfo,
 			this.versionMadeBy = (ushort)madeByInfo;
 			this.versionToExtract = (ushort)versionRequiredToExtract;
 			this.method = method;
+
+			IsUnicodeText = ZipStrings.UseUnicode;
 		}
 
 		/// <summary>

src/ICSharpCode.SharpZipLib/Zip/ZipEntryFactory.cs

@@ -59,27 +59,26 @@ public enum TimeSetting
 		public ZipEntryFactory()
 		{
 			nameTransform_ = new ZipNameTransform();
+			isUnicodeText_ = ZipStrings.UseUnicode;
 		}
 
 		/// <summary>
 		/// Initialise a new instance of <see cref="ZipEntryFactory"/> using the specified <see cref="TimeSetting"/>
 		/// </summary>
 		/// <param name="timeSetting">The <see cref="TimeSetting">time setting</see> to use when creating <see cref="ZipEntry">Zip entries</see>.</param>
-		public ZipEntryFactory(TimeSetting timeSetting)
+		public ZipEntryFactory(TimeSetting timeSetting): this()
 		{
 			timeSetting_ = timeSetting;
-			nameTransform_ = new ZipNameTransform();
 		}
 
 		/// <summary>
 		/// Initialise a new instance of <see cref="ZipEntryFactory"/> using the specified <see cref="DateTime"/>
 		/// </summary>
 		/// <param name="time">The time to set all <see cref="ZipEntry.DateTime"/> values to.</param>
-		public ZipEntryFactory(DateTime time)
+		public ZipEntryFactory(DateTime time): this()
 		{
 			timeSetting_ = TimeSetting.Fixed;
 			FixedDateTime = time;
-			nameTransform_ = new ZipNameTransform();
 		}
 
 		#endregion

src/ICSharpCode.SharpZipLib/Zip/ZipOutputStream.cs

@@ -98,7 +98,7 @@ public bool IsFinished {
 		public void SetComment(string comment)
 		{
 			// TODO: Its not yet clear how to handle unicode comments here.
-			byte[] commentBytes = ZipConstants.ConvertToArray(comment);
+			byte[] commentBytes = ZipStrings.ConvertToArray(comment);
 			if (commentBytes.Length > 0xffff) {
 				throw new ArgumentOutOfRangeException(nameof(comment));
 			}
@@ -320,7 +320,7 @@ public void PutNextEntry(ZipEntry entry)
 				}
 			}
 
-			byte[] name = ZipConstants.ConvertToArray(entry.Flags, entry.Name);
+			byte[] name = ZipStrings.ConvertToArray(entry.Flags, entry.Name);
 
 			if (name.Length > 0xFFFF) {
 				throw new ZipException("Entry name too long.");
@@ -669,7 +669,7 @@ public override void Finish()
 					WriteLeInt((int)entry.Size);
 				}
 
-				byte[] name = ZipConstants.ConvertToArray(entry.Flags, entry.Name);
+				byte[] name = ZipStrings.ConvertToArray(entry.Flags, entry.Name);
 
 				if (name.Length > 0xffff) {
 					throw new ZipException("Name too long.");
@@ -705,7 +705,7 @@ public override void Finish()
 
 				byte[] entryComment =
 					(entry.Comment != null) ?
-					ZipConstants.ConvertToArray(entry.Flags, entry.Comment) :
+					ZipStrings.ConvertToArray(entry.Flags, entry.Comment) :
 					new byte[0];
 
 				if (entryComment.Length > 0xffff) {