From d055b34efec7aafcf95a97ec2b4d671826469464 Mon Sep 17 00:00:00 2001
From: Daniel Nash <dnash2k@gmail.com>
Date: Mon, 20 Jul 2020 12:57:39 -0400
Subject: [PATCH] Ensure TarArchive enumerates all entries

While enumerating the entries of a tar file and writing their contents
to disk using TarArchive, it was discovered TarArchive was not properly
discarding padding bytes in the last block of each entry. TarArchive was
sometimes able to recover depending on the number of padding bytes due
to the logic it uses to find the next entry header, but not always.

TarArchive was changed to use TarReadOnlySubStream when opening entries
and TarReadOnlySubstream was changed to ensure all an entry's blocks are
read when it is being disposed.

Fixes adamhathcock/sharpcompress#524
---
 src/SharpCompress/Common/Tar/TarFilePart.cs   |  2 +-
 .../Common/Tar/TarReadOnlySubStream.cs        | 22 +++++-----
 .../SharpCompress.Test/Tar/TarArchiveTests.cs | 40 +++++++++++++++++++
 3 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/src/SharpCompress/Common/Tar/TarFilePart.cs b/src/SharpCompress/Common/Tar/TarFilePart.cs
index f1a56216..2e11db3d 100644
--- a/src/SharpCompress/Common/Tar/TarFilePart.cs
+++ b/src/SharpCompress/Common/Tar/TarFilePart.cs
@@ -24,7 +24,7 @@ namespace SharpCompress.Common.Tar
             if (_seekableStream != null)
             {
                 _seekableStream.Position = Header.DataStartPosition!.Value;
-                return new ReadOnlySubStream(_seekableStream, Header.Size);
+                return new TarReadOnlySubStream(_seekableStream, Header.Size);
             }
             return Header.PackedStream;
         }
diff --git a/src/SharpCompress/Common/Tar/TarReadOnlySubStream.cs b/src/SharpCompress/Common/Tar/TarReadOnlySubStream.cs
index ed808d42..f0782c6e 100644
--- a/src/SharpCompress/Common/Tar/TarReadOnlySubStream.cs
+++ b/src/SharpCompress/Common/Tar/TarReadOnlySubStream.cs
@@ -20,22 +20,24 @@ namespace SharpCompress.Common.Tar
             {
                 return;
             }
+
             _isDisposed = true;
+
             if (disposing)
             {
-                long skipBytes = _amountRead % 512;
-                if (skipBytes == 0)
+                // Ensure we read all remaining blocks for this entry.
+                Stream.Skip(BytesLeftToRead);
+                _amountRead += BytesLeftToRead;
+
+                // If the last block wasn't a full 512 bytes, skip the remaining padding bytes.
+                var bytesInLastBlock = _amountRead % 512;
+
+                if (bytesInLastBlock != 0)
                 {
-                    return;
+                    Stream.Skip(512 - bytesInLastBlock);
                 }
-                skipBytes = 512 - skipBytes;
-                if (skipBytes == 0)
-                {
-                    return;
-                }
-                var buffer = new byte[skipBytes];
-                Stream.ReadFully(buffer);
             }
+
             base.Dispose(disposing);
         }
 
diff --git a/tests/SharpCompress.Test/Tar/TarArchiveTests.cs b/tests/SharpCompress.Test/Tar/TarArchiveTests.cs
index a03bb64e..69ae3058 100644
--- a/tests/SharpCompress.Test/Tar/TarArchiveTests.cs
+++ b/tests/SharpCompress.Test/Tar/TarArchiveTests.cs
@@ -248,5 +248,45 @@ namespace SharpCompress.Test.Tar
                 }
             }
         }
+
+        [Fact]
+        public void Tar_Read_One_At_A_Time()
+        {
+            var archiveEncoding = new ArchiveEncoding { Default = Encoding.UTF8, };
+            var tarWriterOptions = new TarWriterOptions(CompressionType.None, true) { ArchiveEncoding = archiveEncoding, };
+            var testBytes = Encoding.UTF8.GetBytes("This is a test.");
+
+            using (var memoryStream = new MemoryStream())
+            {
+                using (var tarWriter = new TarWriter(memoryStream, tarWriterOptions))
+                using (var testFileStream = new MemoryStream(testBytes))
+                {
+                    tarWriter.Write("test1.txt", testFileStream);
+                    testFileStream.Position = 0;
+                    tarWriter.Write("test2.txt", testFileStream);
+                }
+
+                memoryStream.Position = 0;
+
+                var numberOfEntries = 0;
+
+                using (var archiveFactory = TarArchive.Open(memoryStream))
+                {
+                    foreach (var entry in archiveFactory.Entries)
+                    {
+                        ++numberOfEntries;
+
+                        using (var tarEntryStream = entry.OpenEntryStream())
+                        using (var testFileStream = new MemoryStream())
+                        {
+                            tarEntryStream.CopyTo(testFileStream);
+                            Assert.Equal(testBytes.Length, testFileStream.Length);
+                        }
+                    }
+                }
+
+                Assert.Equal(2, numberOfEntries);
+            }
+        }
     }
 }