Merge PR #302: Add a leaveOpen parameter to the ZipFile constructor

Used to set the default value of isStreamOwner.

* Add a leaveOpen parameter to the ZipFile constructor, used to set the default value of isStreamOwner
* Add unit tests for the leaveOpen parameter on the ZipFile constructor

Author: Numpsy

src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs

@@ -425,7 +425,25 @@ public ZipFile(string name)
 		/// <exception cref="ZipException">
 		/// The file doesn't contain a valid zip archive.
 		/// </exception>
-		public ZipFile(FileStream file)
+		public ZipFile(FileStream file) :
+			this(file, false)
+		{
+			
+		}
+
+		/// <summary>
+		/// Opens a Zip file reading the given <see cref="FileStream"/>.
+		/// </summary>
+		/// <param name="file">The <see cref="FileStream"/> to read archive data from.</param>
+		/// <param name="leaveOpen">true to leave the <see cref="FileStream">file</see> open when the ZipFile is disposed, false to dispose of it</param>
+		/// <exception cref="ArgumentNullException">The supplied argument is null.</exception>
+		/// <exception cref="IOException">
+		/// An i/o error occurs.
+		/// </exception>
+		/// <exception cref="ZipException">
+		/// The file doesn't contain a valid zip archive.
+		/// </exception>
+		public ZipFile(FileStream file, bool leaveOpen)
 		{
 			if (file == null)
 			{
@@ -439,7 +457,7 @@ public ZipFile(FileStream file)
 
 			baseStream_ = file;
 			name_ = file.Name;
-			isStreamOwner = true;
+			isStreamOwner = !leaveOpen;
 
 			try
 			{
@@ -468,7 +486,30 @@ public ZipFile(FileStream file)
 		/// <exception cref="ArgumentNullException">
 		/// The <see cref="Stream">stream</see> argument is null.
 		/// </exception>
-		public ZipFile(Stream stream)
+		public ZipFile(Stream stream) :
+			this(stream, false)
+		{
+			
+		}
+
+		/// <summary>
+		/// Opens a Zip file reading the given <see cref="Stream"/>.
+		/// </summary>
+		/// <param name="stream">The <see cref="Stream"/> to read archive data from.</param>
+		/// <param name="leaveOpen">true to leave the <see cref="Stream">stream</see> open when the ZipFile is disposed, false to dispose of it</param>
+		/// <exception cref="IOException">
+		/// An i/o error occurs
+		/// </exception>
+		/// <exception cref="ZipException">
+		/// The stream doesn't contain a valid zip archive.<br/>
+		/// </exception>
+		/// <exception cref="ArgumentException">
+		/// The <see cref="Stream">stream</see> doesnt support seeking.
+		/// </exception>
+		/// <exception cref="ArgumentNullException">
+		/// The <see cref="Stream">stream</see> argument is null.
+		/// </exception>
+		public ZipFile(Stream stream, bool leaveOpen)
 		{
 			if (stream == null)
 			{
@@ -481,7 +522,7 @@ public ZipFile(Stream stream)
 			}
 
 			baseStream_ = stream;
-			isStreamOwner = true;
+			isStreamOwner = !leaveOpen;
 
 			if (baseStream_.Length > 0)
 			{

test/ICSharpCode.SharpZipLib.Tests/TestSupport/Streams.cs

@@ -99,6 +99,74 @@ public bool IsDisposed
 		#endregion Instance Fields
 	}
 
+	/// <summary>
+	/// An extended <see cref="FileStream">file stream</see>
+	/// that tracks closing and disposing
+	/// </summary>
+	public class TrackedFileStream : FileStream
+	{
+		/// <summary>
+		/// Initializes a new instance of the <see cref="TrackedMemoryStream"/> class.
+		/// </summary>
+		/// <param name="buffer">The buffer.</param>
+		public TrackedFileStream(string path, FileMode mode = FileMode.Open, FileAccess access = FileAccess.Read)
+			: base(path, mode, access)
+		{
+		}
+
+		/// <summary>
+		/// Releases the unmanaged resources used by the <see cref="T:System.IO.MemoryStream"/> class and optionally releases the managed resources.
+		/// </summary>
+		/// <param name="disposing">true to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
+		protected override void Dispose(bool disposing)
+		{
+			isDisposed_ = true;
+			base.Dispose(disposing);
+		}
+
+		/// <summary>
+		/// Closes the current stream and releases any resources (such as sockets and file handles) associated with the current stream.
+		/// </summary>
+		public override void Close()
+		{
+			if (isClosed_)
+			{
+				throw new InvalidOperationException("Already closed");
+			}
+
+			isClosed_ = true;
+			base.Close();
+		}
+
+		/// <summary>
+		/// Gets a value indicating whether this instance is closed.
+		/// </summary>
+		/// <value><c>true</c> if this instance is closed; otherwise, <c>false</c>.</value>
+		public bool IsClosed
+		{
+			get { return isClosed_; }
+		}
+
+		/// <summary>
+		/// Gets a value indicating whether this instance is disposed.
+		/// </summary>
+		/// <value>
+		/// 	<c>true</c> if this instance is disposed; otherwise, <c>false</c>.
+		/// </value>
+		public bool IsDisposed
+		{
+			get { return isDisposed_; }
+		}
+
+		#region Instance Fields
+
+		private bool isDisposed_;
+
+		private bool isClosed_;
+
+		#endregion Instance Fields
+	}
+
 	/// <summary>
 	/// A <see cref="Stream"/> that cannot seek.
 	/// </summary>

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

@@ -1138,5 +1138,234 @@ public void UnreferencedZipFileClosingPartialStream()
 
 			s.ReadByte();
 		}
+
+		/// <summary>
+		/// Check that input stream is closed when IsStreamOwner is true (default), or leaveOpen is false
+		/// </summary>
+		[Test]
+		[Category("Zip")]
+		public void StreamClosedWhenOwner()
+		{
+			var ms = new MemoryStream();
+			MakeZipFile(ms, false, "StreamClosedWhenOwner", 1, 10, "test");
+			ms.Seek(0, SeekOrigin.Begin);
+			var zipData = ms.ToArray();
+
+			// Stream should be closed when leaveOpen is unspecified
+			{
+				var inMemoryZip = new TrackedMemoryStream(zipData);
+				Assert.IsFalse(inMemoryZip.IsClosed, "Input stream should NOT be closed");
+
+				using (var zipFile = new ZipFile(inMemoryZip))
+				{
+					Assert.IsTrue(zipFile.IsStreamOwner, "Should be stream owner by default");
+				}
+
+				Assert.IsTrue(inMemoryZip.IsClosed, "Input stream should be closed by default");
+			}
+
+			// Stream should be closed when leaveOpen is false
+			{
+				var inMemoryZip = new TrackedMemoryStream(zipData);
+				Assert.IsFalse(inMemoryZip.IsClosed, "Input stream should NOT be closed");
+
+				using (var zipFile = new ZipFile(inMemoryZip, false))
+				{
+					Assert.IsTrue(zipFile.IsStreamOwner, "Should be stream owner when leaveOpen is false");
+				}
+
+				Assert.IsTrue(inMemoryZip.IsClosed, "Input stream should be closed when leaveOpen is false");
+			}
+		}
+
+		/// <summary>
+		/// Check that input stream is not closed when IsStreamOwner is false;
+		/// </summary>
+		[Test]
+		[Category("Zip")]
+		public void StreamNotClosedWhenNotOwner()
+		{
+			var ms = new TrackedMemoryStream();
+			MakeZipFile(ms, false, "StreamNotClosedWhenNotOwner", 1, 10, "test");
+			ms.Seek(0, SeekOrigin.Begin);
+
+			Assert.IsFalse(ms.IsClosed, "Input stream should NOT be closed");
+
+			// Stream should not be closed when leaveOpen is true
+			{
+				using (var zipFile = new ZipFile(ms, true))
+				{
+					Assert.IsFalse(zipFile.IsStreamOwner, "Should NOT be stream owner when leaveOpen is true");
+				}
+
+				Assert.IsFalse(ms.IsClosed, "Input stream should NOT be closed when leaveOpen is true");
+			}
+
+			ms.Seek(0, SeekOrigin.Begin);
+
+			// Stream should not be closed when IsStreamOwner is set to false after opening
+			{
+				using (var zipFile = new ZipFile(ms, false))
+				{
+					Assert.IsTrue(zipFile.IsStreamOwner, "Should be stream owner when leaveOpen is false");
+					zipFile.IsStreamOwner = false;
+					Assert.IsFalse(zipFile.IsStreamOwner, "Should be able to set IsStreamOwner to false");
+				}
+
+				Assert.IsFalse(ms.IsClosed, "Input stream should NOT be closed when IsStreamOwner is false");
+			}
+		}
+
+		/// <summary>
+		/// Check that input file is closed when IsStreamOwner is true (default), or leaveOpen is false
+		/// </summary>
+		[Test]
+		[Category("Zip")]
+		public void FileStreamClosedWhenOwner()
+		{
+			string tempFile = GetTempFilePath();
+			Assert.IsNotNull(tempFile, "No permission to execute this test?");
+
+			tempFile = Path.Combine(tempFile, "SharpZipFileStreamClosedWhenOwnerTest.Zip");
+			if (File.Exists(tempFile))
+			{
+				File.Delete(tempFile);
+			}
+
+			MakeZipFile(tempFile, "FileStreamClosedWhenOwner", 2, 10, "test");
+
+			// Stream should be closed when leaveOpen is unspecified
+			{
+				var fileStream = new TrackedFileStream(tempFile);
+				Assert.IsFalse(fileStream.IsClosed, "Input file should NOT be closed");
+
+				using (var zipFile = new ZipFile(fileStream))
+				{
+					Assert.IsTrue(zipFile.IsStreamOwner, "Should be stream owner by default");
+				}
+
+				Assert.IsTrue(fileStream.IsClosed, "Input stream should be closed by default");
+			}
+
+			// Stream should be closed when leaveOpen is false
+			{
+				var fileStream = new TrackedFileStream(tempFile);
+				Assert.IsFalse(fileStream.IsClosed, "Input stream should NOT be closed");
+
+				using (var zipFile = new ZipFile(fileStream, false))
+				{
+					Assert.IsTrue(zipFile.IsStreamOwner, "Should be stream owner when leaveOpen is false");
+				}
+
+				Assert.IsTrue(fileStream.IsClosed, "Input stream should be closed when leaveOpen is false");
+			}
+
+			File.Delete(tempFile);
+		}
+
+		/// <summary>
+		/// Check that input file is not closed when IsStreamOwner is false;
+		/// </summary>
+		[Test]
+		[Category("Zip")]
+		public void FileStreamNotClosedWhenNotOwner()
+		{
+			string tempFile = GetTempFilePath();
+			Assert.IsNotNull(tempFile, "No permission to execute this test?");
+
+			tempFile = Path.Combine(tempFile, "SharpZipFileStreamNotClosedWhenNotOwner.Zip");
+			if (File.Exists(tempFile))
+			{
+				File.Delete(tempFile);
+			}
+
+			MakeZipFile(tempFile, "FileStreamClosedWhenOwner", 2, 10, "test");
+
+			// Stream should not be closed when leaveOpen is true
+			{
+				using (var fileStream = new TrackedFileStream(tempFile))
+				{
+					Assert.IsFalse(fileStream.IsClosed, "Input file should NOT be closed");
+
+					using (var zipFile = new ZipFile(fileStream, true))
+					{
+						Assert.IsFalse(zipFile.IsStreamOwner, "Should NOT be stream owner when leaveOpen is true");
+					}
+
+					Assert.IsFalse(fileStream.IsClosed, "Input stream should NOT be closed when leaveOpen is true");
+				}
+			}
+
+			// Stream should not be closed when IsStreamOwner is set to false after opening
+			{
+				using (var fileStream = new TrackedFileStream(tempFile))
+				{
+					Assert.IsFalse(fileStream.IsClosed, "Input file should NOT be closed");
+
+					using (var zipFile = new ZipFile(fileStream, false))
+					{
+						Assert.IsTrue(zipFile.IsStreamOwner, "Should be stream owner when leaveOpen is false");
+						zipFile.IsStreamOwner = false;
+						Assert.IsFalse(zipFile.IsStreamOwner, "Should be able to set IsStreamOwner to false");
+					}
+
+					Assert.IsFalse(fileStream.IsClosed, "Input stream should NOT be closed when leaveOpen is true");
+				}
+			}
+
+			File.Delete(tempFile);
+		}
+
+		/// <summary>
+		/// Check that input stream is closed when construction fails and leaveOpen is false
+		/// </summary>
+		[Test]
+		public void StreamClosedOnError()
+		{
+			var ms = new TrackedMemoryStream(new byte[32]);
+
+			Assert.IsFalse(ms.IsClosed, "Underlying stream should NOT be closed initially");
+			bool blewUp = false;
+			try
+			{
+				using (var zipFile = new ZipFile(ms, false))
+				{
+					Assert.Fail("Exception not thrown");
+				}
+			}
+			catch
+			{
+				blewUp = true;
+			}
+
+			Assert.IsTrue(blewUp, "Should have failed to load the file");
+			Assert.IsTrue(ms.IsClosed, "Underlying stream should be closed");
+		}
+
+		/// <summary>
+		/// Check that input stream is not closed when construction fails and leaveOpen is true
+		/// </summary>
+		[Test]
+		public void StreamNotClosedOnError()
+		{
+			var ms = new TrackedMemoryStream(new byte[32]);
+
+			Assert.IsFalse(ms.IsClosed, "Underlying stream should NOT be closed initially");
+			bool blewUp = false;
+			try
+			{
+				using (var zipFile = new ZipFile(ms, true))
+				{
+					Assert.Fail("Exception not thrown");
+				}
+			}
+			catch
+			{
+				blewUp = true;
+			}
+
+			Assert.IsTrue(blewUp, "Should have failed to load the file");
+			Assert.IsFalse(ms.IsClosed, "Underlying stream should NOT be closed");
+		}
 	}
 }