Merge PR# 311: Set isStreamOwner from ZipFile constructor in FastZip.…

…ExtractZip

* Change FastZip.ExtractZip to use the new ZipFile constructor to set isStreamOwner, rather than setting the property after construction.

* Add unit tests for testing that FastZip.Extract handles stream disposal a bit better when handling a corrupt zip file

src/ICSharpCode.SharpZipLib/Zip/FastZip.cs

@@ -464,13 +464,13 @@ public void ExtractZip(Stream inputStream, string targetDirectory,
 			directoryFilter_ = new NameFilter(directoryFilter);
 			restoreDateTimeOnExtract_ = restoreDateTime;
 
-			using (zipFile_ = new ZipFile(inputStream))
+			using (zipFile_ = new ZipFile(inputStream, !isStreamOwner))
 			{
 				if (password_ != null)
 				{
 					zipFile_.Password = password_;
 				}
-				zipFile_.IsStreamOwner = isStreamOwner;
+
 				System.Collections.IEnumerator enumerator = zipFile_.GetEnumerator();
 				while (continueRunning_ && enumerator.MoveNext())
 				{

test/ICSharpCode.SharpZipLib.Tests/Zip/FastZipHandling.cs

@@ -447,5 +447,61 @@ void CreateTestFile(string archiveFile, string contentPath)
 				Directory.Delete(tempPath, true);
 			}
 		}
+
+		/// <summary>
+		/// Check that the input stream is not closed on error when isStreamOwner is false
+		/// </summary>
+		[Test]
+		public void StreamNotClosedOnError()
+		{
+			// test paths
+			string tempFilePath = GetTempFilePath();
+			Assert.IsNotNull(tempFilePath, "No permission to execute this test?");
+
+			var tempFolderPath = Path.Combine(tempFilePath, Path.GetRandomFileName());
+			Assert.That(Directory.Exists(tempFolderPath), Is.False, "Temp folder path should not exist");
+
+			// memory that isn't a valid zip
+			var ms = new TrackedMemoryStream(new byte[32]);
+			Assert.IsFalse(ms.IsClosed, "Underlying stream should NOT be closed initially");
+
+			// Try to extract
+			var fastZip = new FastZip();
+			fastZip.CreateEmptyDirectories = true;
+
+			Assert.Throws<ZipException>(() => fastZip.ExtractZip(ms, tempFolderPath, FastZip.Overwrite.Always, null, "a", "b", false, false), "Should throw when extracting an invalid file");
+			Assert.IsFalse(ms.IsClosed, "inputStream stream should NOT be closed when isStreamOwner is false");
+
+			// test folder should not have been created on error
+			Assert.That(Directory.Exists(tempFolderPath), Is.False, "Temp folder path should still not exist");
+		}
+
+		/// <summary>
+		/// Check that the input stream is closed on error when isStreamOwner is true
+		/// </summary>
+		[Test]
+		public void StreamClosedOnError()
+		{
+			// test paths
+			string tempFilePath = GetTempFilePath();
+			Assert.IsNotNull(tempFilePath, "No permission to execute this test?");
+
+			var tempFolderPath = Path.Combine(tempFilePath, Path.GetRandomFileName());
+			Assert.That(Directory.Exists(tempFolderPath), Is.False, "Temp folder path should not exist");
+
+			// memory that isn't a valid zip
+			var ms = new TrackedMemoryStream(new byte[32]);
+			Assert.IsFalse(ms.IsClosed, "Underlying stream should NOT be closed initially");
+
+			// Try to extract
+			var fastZip = new FastZip();
+			fastZip.CreateEmptyDirectories = true;
+
+			Assert.Throws<ZipException>(() => fastZip.ExtractZip(ms, tempFolderPath, FastZip.Overwrite.Always, null, "a", "b", false, true), "Should throw when extracting an invalid file");
+			Assert.IsTrue(ms.IsClosed, "inputStream stream should be closed when isStreamOwner is true");
+
+			// test folder should not have been created on error
+			Assert.That(Directory.Exists(tempFolderPath), Is.False, "Temp folder path should still not exist");
+		}
 	}
 }