Merge PR #326: Dispose TarArchive output streams in case of an exception

* Refactor TarArchive.ExtractEntry() to use using blocks for stream disposal
* Unit test to check that TarArchive.ExtractContents() doesn't leak file handles when an exception occurs

Author: Numpsy

src/ICSharpCode.SharpZipLib/Tar/TarArchive.cs

@@ -623,20 +623,32 @@ private void ExtractEntry(string destDir, TarEntry entry)
 
 				if (process)
 				{
-					bool asciiTrans = false;
-
-					Stream outputStream = File.Create(destFile);
-					if (this.asciiTranslate)
+					using (var outputStream = File.Create(destFile))
 					{
-						asciiTrans = !IsBinary(destFile);
+						if (this.asciiTranslate)
+						{
+							// May need to translate the file.
+							ExtractAndTranslateEntry(destFile, outputStream);
+						}
+						else
+						{
+							// If translation is disabled, just copy the entry across directly.
+							tarIn.CopyEntryContents(outputStream);
+						}
 					}
+				}
+			}
+		}
 
-					StreamWriter outw = null;
-					if (asciiTrans)
-					{
-						outw = new StreamWriter(outputStream);
-					}
+		// Extract a TAR entry, and perform an ASCII translation if required.
+		private void ExtractAndTranslateEntry(string destFile, Stream outputStream)
+		{
+			bool asciiTrans = !IsBinary(destFile);
 
+			if (asciiTrans)
+			{
+				using (var outw = new StreamWriter(outputStream, new UTF8Encoding(false), 1024, true))
+				{
 					byte[] rdbuf = new byte[32 * 1024];
 
 					while (true)
@@ -648,34 +660,23 @@ private void ExtractEntry(string destDir, TarEntry entry)
 							break;
 						}
 
-						if (asciiTrans)
+						for (int off = 0, b = 0; b < numRead; ++b)
 						{
-							for (int off = 0, b = 0; b < numRead; ++b)
+							if (rdbuf[b] == 10)
 							{
-								if (rdbuf[b] == 10)
-								{
-									string s = Encoding.ASCII.GetString(rdbuf, off, (b - off));
-									outw.WriteLine(s);
-									off = b + 1;
-								}
+								string s = Encoding.ASCII.GetString(rdbuf, off, (b - off));
+								outw.WriteLine(s);
+								off = b + 1;
 							}
 						}
-						else
-						{
-							outputStream.Write(rdbuf, 0, numRead);
-						}
-					}
-
-					if (asciiTrans)
-					{
-						outw.Dispose();
-					}
-					else
-					{
-						outputStream.Dispose();
 					}
 				}
 			}
+			else
+			{
+				// No translation required.
+				tarIn.CopyEntryContents(outputStream);
+			}
 		}
 
 		/// <summary>

test/ICSharpCode.SharpZipLib.Tests/Tar/TarTests.cs

@@ -1,4 +1,5 @@
 using ICSharpCode.SharpZipLib.Tar;
+using ICSharpCode.SharpZipLib.GZip;
 using ICSharpCode.SharpZipLib.Tests.TestSupport;
 using NUnit.Framework;
 using System;
@@ -787,5 +788,51 @@ public void SingleLargeEntry()
 				}
 			);
 		}
+
+		// Test for corruption issue described @ https://github.com/icsharpcode/SharpZipLib/issues/321
+		[Test]
+		[Category("Tar")]
+		public void ExtractingCorruptTarShouldntLeakFiles()
+		{
+			using (var memoryStream = new MemoryStream())
+			{
+				//Create a tar.gz in the output stream
+				using (var gzipStream = new GZipOutputStream(memoryStream))
+				{
+					gzipStream.IsStreamOwner = false;
+
+					using (var tarOut = TarArchive.CreateOutputTarArchive(gzipStream))
+					using (var dummyFile = Utils.GetDummyFile(32000))
+					{
+						tarOut.IsStreamOwner = false;
+						tarOut.WriteEntry(TarEntry.CreateEntryFromFile(dummyFile.Filename), false);
+					}
+				}
+
+				// corrupt archive - make sure the file still has more than one block
+				memoryStream.SetLength(16000);
+				memoryStream.Seek(0, SeekOrigin.Begin);
+
+				// try to extract
+				using (var gzipStream = new GZipInputStream(memoryStream))
+				{
+					string tempDirName;
+					gzipStream.IsStreamOwner = false;
+
+					using (var tempDir = new Utils.TempDir())
+					{
+						tempDirName = tempDir.Fullpath;
+
+						using (var tarIn = TarArchive.CreateInputTarArchive(gzipStream))
+						{
+							tarIn.IsStreamOwner = false;
+							Assert.Throws<SharpZipBaseException>(() => tarIn.ExtractContents(tempDir.Fullpath));
+						}
+					}
+
+					Assert.That(Directory.Exists(tempDirName), Is.False, "Temporary folder should have been removed");
+				}	
+			}
+		}
 	}
 }