refactor for readability and more specific errors

Author: piksel

src/ICSharpCode.SharpZipLib/Zip/ZipConstants.cs

@@ -231,6 +231,21 @@ public enum GeneralBitFlags
 		/// </summary>
 		ReservedPkware15 = 0x8000
 	}
+	
+	/// <summary>
+	/// Helpers for <see cref="GeneralBitFlags"/>
+	/// </summary>
+	public static class GeneralBitFlagsExtensions
+	{
+		/// <summary>
+		/// This is equivalent of <see cref="Enum.HasFlag"/> in .NET Core, but since the .NET FW
+		/// version is really slow (due to un-/boxing and reflection)  we use this wrapper.
+		/// </summary>
+		/// <param name="flagData"></param>
+		/// <param name="flag"></param>
+		/// <returns></returns>
+		public static bool Includes(this GeneralBitFlags flagData, GeneralBitFlags flag) => (flag & flagData) != 0;
+	}
 
 	#endregion Enumerations
 

src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs

@@ -1111,13 +1111,12 @@ private long TestLocalHeader(ZipEntry entry, HeaderTest tests)
 
 				if (signature != ZipConstants.LocalHeaderSignature)
 				{
-					throw new ZipException(string.Format("Wrong local header signature at 0x{0:x}, expected 0x{1:x8}, actual 0x{2:x8}",
-						entryAbsOffset, ZipConstants.LocalHeaderSignature, signature));
+					throw new ZipException($"Wrong local header signature at 0x{entryAbsOffset:x}, expected 0x{ZipConstants.LocalHeaderSignature:x8}, actual 0x{signature:x8}");
 				}
 
 				var extractVersion = (short)(ReadLEUshort() & 0x00ff);
-				var localFlags = (short)ReadLEUshort();
-				var compressionMethod = (short)ReadLEUshort();
+				var localFlags = (GeneralBitFlags)ReadLEUshort();
+				var compressionMethod = (CompressionMethod)ReadLEUshort();
 				var fileTime = (short)ReadLEUshort();
 				var fileDate = (short)ReadLEUshort();
 				uint crcValue = ReadLEUint();
@@ -1135,15 +1134,15 @@ private long TestLocalHeader(ZipEntry entry, HeaderTest tests)
 				var localExtraData = new ZipExtraData(extraData);
 
 				// Extra data / zip64 checks
-				if (localExtraData.Find(1))
+				if (localExtraData.Find(headerID: 1))
 				{
 					// 2010-03-04 Forum 10512: removed checks for version >= ZipConstants.VersionZip64
 					// and size or compressedSize = MaxValue, due to rogue creators.
 
 					size = localExtraData.ReadLong();
 					compressedSize = localExtraData.ReadLong();
 
-					if ((localFlags & (int)GeneralBitFlags.Descriptor) != 0)
+					if (localFlags.Includes(GeneralBitFlags.Descriptor))
 					{
 						// These may be valid if patched later
 						if ((size != -1) && (size != entry.Size))
@@ -1176,15 +1175,17 @@ private long TestLocalHeader(ZipEntry entry, HeaderTest tests)
 							throw new ZipException("Compression method not supported");
 						}
 
-						if ((extractVersion > ZipConstants.VersionMadeBy)
-							|| ((extractVersion > 20) && (extractVersion < ZipConstants.VersionZip64)))
+						if (extractVersion > ZipConstants.VersionMadeBy
+							|| (extractVersion > 20 && extractVersion < ZipConstants.VersionZip64))
 						{
-							throw new ZipException(string.Format("Version required to extract this entry not supported ({0})", extractVersion));
+							throw new ZipException($"Version required to extract this entry not supported ({extractVersion})");
 						}
 
-						if ((localFlags & (int)(GeneralBitFlags.Patched | GeneralBitFlags.StrongEncryption | GeneralBitFlags.EnhancedCompress | GeneralBitFlags.HeaderMasked)) != 0)
+						const GeneralBitFlags notSupportedFlags = GeneralBitFlags.Patched | GeneralBitFlags.StrongEncryption |
+						                        GeneralBitFlags.EnhancedCompress | GeneralBitFlags.HeaderMasked;
+						if (localFlags.Includes(notSupportedFlags))
 						{
-							throw new ZipException("The library does not support the zip version required to extract this entry");
+							throw new ZipException($"The library does not support the zip features required to extract this entry ({localFlags & notSupportedFlags:F})");
 						}
 					}
 				}
@@ -1208,53 +1209,53 @@ private long TestLocalHeader(ZipEntry entry, HeaderTest tests)
 						(extractVersion != 63)
 						)
 					{
-						throw new ZipException(string.Format("Version required to extract this entry is invalid ({0})", extractVersion));
+						throw new ZipException($"Version required to extract this entry is invalid ({extractVersion})");
 					}
 
 					var localEncoding = _stringCodec.ZipInputEncoding(localFlags);
 
 					// Local entry flags dont have reserved bit set on.
-					if ((localFlags & (int)(GeneralBitFlags.ReservedPKware4 | GeneralBitFlags.ReservedPkware14 | GeneralBitFlags.ReservedPkware15)) != 0)
+					if (localFlags.Includes(GeneralBitFlags.ReservedPKware4 | GeneralBitFlags.ReservedPkware14 | GeneralBitFlags.ReservedPkware15))
 					{
 						throw new ZipException("Reserved bit flags cannot be set.");
 					}
 
 					// Encryption requires extract version >= 20
-					if (((localFlags & (int)GeneralBitFlags.Encrypted) != 0) && (extractVersion < 20))
+					if (localFlags.Includes(GeneralBitFlags.Encrypted) && extractVersion < 20)
 					{
-						throw new ZipException(string.Format("Version required to extract this entry is too low for encryption ({0})", extractVersion));
+						throw new ZipException($"Version required to extract this entry is too low for encryption ({extractVersion})");
 					}
 
 					// Strong encryption requires encryption flag to be set and extract version >= 50.
-					if ((localFlags & (int)GeneralBitFlags.StrongEncryption) != 0)
+					if (localFlags.Includes(GeneralBitFlags.StrongEncryption))
 					{
-						if ((localFlags & (int)GeneralBitFlags.Encrypted) == 0)
+						if (!localFlags.Includes(GeneralBitFlags.Encrypted))
 						{
 							throw new ZipException("Strong encryption flag set but encryption flag is not set");
 						}
 
 						if (extractVersion < 50)
 						{
-							throw new ZipException(string.Format("Version required to extract this entry is too low for encryption ({0})", extractVersion));
+							throw new ZipException($"Version required to extract this entry is too low for encryption ({extractVersion})");
 						}
 					}
 
 					// Patched entries require extract version >= 27
-					if (((localFlags & (int)GeneralBitFlags.Patched) != 0) && (extractVersion < 27))
+					if (localFlags.Includes(GeneralBitFlags.Patched) && extractVersion < 27)
 					{
-						throw new ZipException(string.Format("Patched data requires higher version than ({0})", extractVersion));
+						throw new ZipException($"Patched data requires higher version than ({extractVersion})");
 					}
 
 					// Central header flags match local entry flags.
-					if (localFlags != entry.Flags)
+					if (!localFlags.Equals((GeneralBitFlags)entry.Flags))
 					{
-						throw new ZipException("Central header/local header flags mismatch");
+						throw new ZipException($"Central header/local header flags mismatch ({(GeneralBitFlags)entry.Flags:F} vs {localFlags:F})");
 					}
 
 					// Central header compression method matches local entry
-					if (entry.CompressionMethodForHeader != (CompressionMethod)compressionMethod)
+					if (entry.CompressionMethodForHeader != compressionMethod)
 					{
-						throw new ZipException("Central header/local header compression method mismatch");
+						throw new ZipException($"Central header/local header compression method mismatch ({entry.CompressionMethodForHeader:G} vs {compressionMethod:G})");
 					}
 
 					if (entry.Version != extractVersion)
@@ -1263,23 +1264,23 @@ private long TestLocalHeader(ZipEntry entry, HeaderTest tests)
 					}
 
 					// Strong encryption and extract version match
-					if ((localFlags & (int)GeneralBitFlags.StrongEncryption) != 0)
+					if (localFlags.Includes(GeneralBitFlags.StrongEncryption))
 					{
 						if (extractVersion < 62)
 						{
 							throw new ZipException("Strong encryption flag set but version not high enough");
 						}
 					}
 
-					if ((localFlags & (int)GeneralBitFlags.HeaderMasked) != 0)
+					if (localFlags.Includes(GeneralBitFlags.HeaderMasked))
 					{
-						if ((fileTime != 0) || (fileDate != 0))
+						if (fileTime != 0 || fileDate != 0)
 						{
 							throw new ZipException("Header masked set but date/time values non-zero");
 						}
 					}
 
-					if ((localFlags & (int)GeneralBitFlags.Descriptor) == 0)
+					if (!localFlags.Includes(GeneralBitFlags.Descriptor))
 					{
 						if (crcValue != (uint)entry.Crc)
 						{
@@ -1288,8 +1289,8 @@ private long TestLocalHeader(ZipEntry entry, HeaderTest tests)
 					}
 
 					// Crc valid for empty entry.
-					// This will also apply to streamed entries where size isnt known and the header cant be patched
-					if ((size == 0) && (compressedSize == 0))
+					// This will also apply to streamed entries where size isn't known and the header cant be patched
+					if (size == 0 && compressedSize == 0)
 					{
 						if (crcValue != 0)
 						{
@@ -1349,23 +1350,18 @@ private long TestLocalHeader(ZipEntry entry, HeaderTest tests)
 
 				// Size can be verified only if it is known in the local header.
 				// it will always be known in the central header.
-				if (((localFlags & (int)GeneralBitFlags.Descriptor) == 0) ||
+				if (!localFlags.Includes(GeneralBitFlags.Descriptor) ||
 					((size > 0 || compressedSize > 0) && entry.Size > 0))
 				{
-					if ((size != 0)
-						&& (size != entry.Size))
+					if (size != 0 && size != entry.Size)
 					{
-						throw new ZipException(
-							string.Format("Size mismatch between central header({0}) and local header({1})",
-								entry.Size, size));
+						throw new ZipException($"Size mismatch between central header ({entry.Size}) and local header ({size})");
 					}
 
-					if ((compressedSize != 0)
+					if (compressedSize != 0
 						&& (compressedSize != entry.CompressedSize && compressedSize != 0xFFFFFFFF && compressedSize != -1))
 					{
-						throw new ZipException(
-							string.Format("Compressed size mismatch between central header({0}) and local header({1})",
-							entry.CompressedSize, compressedSize));
+						throw new ZipException($"Compressed size mismatch between central header({entry.CompressedSize}) and local header({compressedSize})");
 					}
 				}
 
@@ -3503,20 +3499,16 @@ private void ReadEntries()
 			}
 
 			bool isZip64 = false;
-			bool requireZip64 = false;
-
+			
 			// Check if zip64 header information is required.
-			if ((thisDiskNumber == 0xffff) ||
-				(startCentralDirDisk == 0xffff) ||
-				(entriesForThisDisk == 0xffff) ||
-				(entriesForWholeCentralDir == 0xffff) ||
-				(centralDirSize == 0xffffffff) ||
-				(offsetOfCentralDir == 0xffffffff))
-			{
-				requireZip64 = true;
-			}
-
-			// #357 - always check for the existance of the Zip64 central directory.
+			bool requireZip64 = thisDiskNumber == 0xffff ||
+			                    startCentralDirDisk == 0xffff ||
+			                    entriesForThisDisk == 0xffff ||
+			                    entriesForWholeCentralDir == 0xffff ||
+			                    centralDirSize == 0xffffffff ||
+			                    offsetOfCentralDir == 0xffffffff;
+
+			// #357 - always check for the existence of the Zip64 central directory.
 			// #403 - Take account of the fixed size of the locator when searching.
 			//    Subtract from locatedEndOfCentralDir so that the endLocation is the location of EndOfCentralDirectorySignature,
 			//    rather than the data following the signature.
@@ -3550,7 +3542,7 @@ private void ReadEntries()
 
 				if (sig64 != ZipConstants.Zip64CentralFileHeaderSignature)
 				{
-					throw new ZipException(string.Format("Invalid Zip64 Central directory signature at {0:X}", offset64));
+					throw new ZipException($"Invalid Zip64 Central directory signature at {offset64:X}");
 				}
 
 				// NOTE: Record size = SizeOfFixedFields + SizeOfVariableData - 12.
@@ -3605,8 +3597,11 @@ private void ReadEntries()
 				int extraLen = ReadLEUshort();
 				int commentLen = ReadLEUshort();
 
-				int diskStartNo = ReadLEUshort();  // Not currently used
-				int internalAttributes = ReadLEUshort();  // Not currently used
+				
+				// ReSharper disable once UnusedVariable, Currently unused but needs to be read to offset the stream
+				int diskStartNo = ReadLEUshort();
+				// ReSharper disable once UnusedVariable, Currently unused but needs to be read to offset the stream
+				int internalAttributes = ReadLEUshort();
 
 				uint externalAttributes = ReadLEUint();
 				long offset = ReadLEUint();
@@ -3630,7 +3625,7 @@ private void ReadEntries()
 					ExternalFileAttributes = (int)externalAttributes
 				};
 
-				if ((bitFlags & 8) == 0)
+				if (!entry.HasFlag(GeneralBitFlags.Descriptor))
 				{
 					entry.CryptoCheckValue = (byte)(crc >> 24);
 				}
@@ -3698,15 +3693,15 @@ private Stream CreateAndInitDecryptionStream(Stream baseStream, ZipEntry entry)
 					}
 					int saltLen = entry.AESSaltLen;
 					byte[] saltBytes = new byte[saltLen];
-					int saltIn = StreamUtils.ReadRequestedBytes(baseStream, saltBytes, 0, saltLen);
-					if (saltIn != saltLen)
-						throw new ZipException("AES Salt expected " + saltLen + " got " + saltIn);
-					//
+					int saltIn = StreamUtils.ReadRequestedBytes(baseStream, saltBytes, offset: 0, saltLen);
+					
+					if (saltIn != saltLen) throw new ZipException($"AES Salt expected {saltLen} git {saltIn}");
+					
 					byte[] pwdVerifyRead = new byte[2];
 					StreamUtils.ReadFully(baseStream, pwdVerifyRead);
 					int blockSize = entry.AESKeySize / 8;   // bits to bytes
 
-					var decryptor = new ZipAESTransform(rawPassword_, saltBytes, blockSize, false);
+					var decryptor = new ZipAESTransform(rawPassword_, saltBytes, blockSize, writeMode: false);
 					byte[] pwdVerifyCalc = decryptor.PwdVerifier;
 					if (pwdVerifyCalc[0] != pwdVerifyRead[0] || pwdVerifyCalc[1] != pwdVerifyRead[1])
 						throw new ZipException("Invalid password for AES");
@@ -3719,8 +3714,7 @@ private Stream CreateAndInitDecryptionStream(Stream baseStream, ZipEntry entry)
 			}
 			else
 			{
-				if ((entry.Version < ZipConstants.VersionStrongEncryption)
-					|| (entry.Flags & (int)GeneralBitFlags.StrongEncryption) == 0)
+				if (entry.Version < ZipConstants.VersionStrongEncryption || !entry.HasFlag(GeneralBitFlags.StrongEncryption))
 				{
 					var classicManaged = new PkzipClassicManaged();
 
@@ -3745,31 +3739,29 @@ private Stream CreateAndInitDecryptionStream(Stream baseStream, ZipEntry entry)
 
 		private Stream CreateAndInitEncryptionStream(Stream baseStream, ZipEntry entry)
 		{
-			CryptoStream result = null;
-			if ((entry.Version < ZipConstants.VersionStrongEncryption)
-				|| (entry.Flags & (int)GeneralBitFlags.StrongEncryption) == 0)
-			{
-				var classicManaged = new PkzipClassicManaged();
+			if (entry.Version >= ZipConstants.VersionStrongEncryption &&
+			    entry.HasFlag(GeneralBitFlags.StrongEncryption)) return null;
 
-				OnKeysRequired(entry.Name);
-				if (HaveKeys == false)
-				{
-					throw new ZipException("No password available for encrypted stream");
-				}
+			var classicManaged = new PkzipClassicManaged();
+
+			OnKeysRequired(entry.Name);
+			if (HaveKeys == false)
+			{
+				throw new ZipException("No password available for encrypted stream");
+			}
 
-				// Closing a CryptoStream will close the base stream as well so wrap it in an UncompressedStream
-				// which doesnt do this.
-				result = new CryptoStream(new UncompressedStream(baseStream),
-					classicManaged.CreateEncryptor(key, null), CryptoStreamMode.Write);
+			// Closing a CryptoStream will close the base stream as well so wrap it in an UncompressedStream
+			// which doesnt do this.
+			var result = new CryptoStream(new UncompressedStream(baseStream),
+				classicManaged.CreateEncryptor(key, null), CryptoStreamMode.Write);
 
-				if ((entry.Crc < 0) || (entry.Flags & 8) != 0)
-				{
-					WriteEncryptionHeader(result, entry.DosTime << 16);
-				}
-				else
-				{
-					WriteEncryptionHeader(result, entry.Crc);
-				}
+			if (entry.Crc < 0 || entry.HasFlag(GeneralBitFlags.Descriptor))
+			{
+				WriteEncryptionHeader(result, entry.DosTime << 16);
+			}
+			else
+			{
+				WriteEncryptionHeader(result, entry.Crc);
 			}
 			return result;
 		}
@@ -3792,7 +3784,7 @@ private static void WriteEncryptionHeader(Stream stream, long crcValue)
 				rng.GetBytes(cryptBuffer);
 			}
 			cryptBuffer[11] = (byte)(crcValue >> 24);
-			stream.Write(cryptBuffer, 0, cryptBuffer.Length);
+			stream.Write(cryptBuffer, offset: 0, cryptBuffer.Length);
 		}
 
 		#endregion Internal routines