Merge pull request #1342 from adamhathcock/copilot/fix-tararchive-missing-entries

Fix tar archive enumeration after fully reading entry streams

src/SharpCompress/Common/Tar/TarFilePart.cs

@@ -25,7 +25,7 @@ internal sealed class TarFilePart : FilePart
         if (_seekableStream != null)
         {
             _seekableStream.Position = Header.DataStartPosition ?? 0;
-            return new TarReadOnlySubStream(_seekableStream, Header.Size, false);
+            return new TarReadOnlySubStream(_seekableStream, Header.Size);
         }
         return Header.PackedStream.NotNull();
     }
@@ -36,14 +36,8 @@ internal sealed class TarFilePart : FilePart
     {
         if (_seekableStream != null)
         {
-            var useSyncOverAsync = false;
-#if LEGACY_DOTNET
-            useSyncOverAsync = true;
-#endif
             _seekableStream.Position = Header.DataStartPosition ?? 0;
-            return new ValueTask<Stream?>(
-                new TarReadOnlySubStream(_seekableStream, Header.Size, useSyncOverAsync)
-            );
+            return new ValueTask<Stream?>(new TarReadOnlySubStream(_seekableStream, Header.Size));
         }
         return new ValueTask<Stream?>(Header.PackedStream.NotNull());
     }

src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs

@@ -44,15 +44,7 @@ internal static partial class TarHeaderFactory
                         break;
                     case StreamingMode.Streaming:
                         {
-                            var useSyncOverAsync = false;
-#if LEGACY_DOTNET
-                            useSyncOverAsync = true;
-#endif
-                            header.PackedStream = new TarReadOnlySubStream(
-                                stream,
-                                header.Size,
-                                useSyncOverAsync
-                            );
+                            header.PackedStream = new TarReadOnlySubStream(stream, header.Size);
                         }
                         break;
                     default:

src/SharpCompress/Common/Tar/TarHeaderFactory.cs

@@ -38,11 +38,7 @@ internal static partial class TarHeaderFactory
                         break;
                     case StreamingMode.Streaming:
                         {
-                            header.PackedStream = new TarReadOnlySubStream(
-                                stream,
-                                header.Size,
-                                false
-                            );
+                            header.PackedStream = new TarReadOnlySubStream(stream, header.Size);
                         }
                         break;
                     default:

src/SharpCompress/Common/Tar/TarReadOnlySubStream.cs

@@ -1,5 +1,6 @@
 using System;
 using System.IO;
+using System.Threading.Tasks;
 
 namespace SharpCompress.Common.Tar;
 
@@ -8,9 +9,10 @@ internal class TarReadOnlySubStream : Stream
     private readonly Stream _stream;
 
     private bool _isDisposed;
+    private bool _isPositionedAtNextEntry;
     private long _amountRead;
 
-    public TarReadOnlySubStream(Stream stream, long bytesToRead, bool useSyncOverAsyncDispose)
+    public TarReadOnlySubStream(Stream stream, long bytesToRead)
     {
         _stream = stream;
         BytesLeftToRead = bytesToRead;
@@ -27,27 +29,17 @@ internal class TarReadOnlySubStream : Stream
         _isDisposed = true;
         if (disposing)
         {
-            // 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)
+            if (Utility.UseSyncOverAsyncDispose())
             {
-                if (Utility.UseSyncOverAsyncDispose())
-                {
 #pragma warning disable VSTHRD002 // Avoid problematic synchronous waits
 #pragma warning disable CA2012
-                    _stream.SkipAsync(512 - bytesInLastBlock).GetAwaiter().GetResult();
+                AdvanceToNextHeaderAsync().GetAwaiter().GetResult();
 #pragma warning restore CA2012
 #pragma warning restore VSTHRD002 // Avoid problematic synchronous waits
-                }
-                else
-                {
-                    _stream.Skip(512 - bytesInLastBlock);
-                }
+            }
+            else
+            {
+                AdvanceToNextHeader();
             }
         }
         base.Dispose(disposing);
@@ -63,17 +55,7 @@ internal class TarReadOnlySubStream : Stream
         }
 
         _isDisposed = true;
-        // Ensure we read all remaining blocks for this entry.
-        await _stream.SkipAsync(BytesLeftToRead).ConfigureAwait(false);
-        _amountRead += BytesLeftToRead;
-
-        // If the last block wasn't a full 512 bytes, skip the remaining padding bytes.
-        var bytesInLastBlock = _amountRead % 512;
-
-        if (bytesInLastBlock != 0)
-        {
-            await _stream.SkipAsync(512 - bytesInLastBlock).ConfigureAwait(false);
-        }
+        await AdvanceToNextHeaderAsync().ConfigureAwait(false);
 
         GC.SuppressFinalize(this);
         await base.DisposeAsync().ConfigureAwait(false);
@@ -82,6 +64,54 @@ internal class TarReadOnlySubStream : Stream
 
     private long BytesLeftToRead { get; set; }
 
+    private void AdvanceToNextHeader()
+    {
+        if (_isPositionedAtNextEntry)
+        {
+            return;
+        }
+
+        if (BytesLeftToRead > 0)
+        {
+            _stream.Skip(BytesLeftToRead);
+            _amountRead += BytesLeftToRead;
+            BytesLeftToRead = 0;
+        }
+
+        // Tar entry data is padded to 512-byte blocks, so callers that read to EOF
+        // should still leave the shared archive stream positioned at the next header.
+        var bytesInLastBlock = _amountRead % 512;
+        if (bytesInLastBlock != 0)
+        {
+            _stream.Skip(512 - bytesInLastBlock);
+        }
+
+        _isPositionedAtNextEntry = true;
+    }
+
+    private async ValueTask AdvanceToNextHeaderAsync()
+    {
+        if (_isPositionedAtNextEntry)
+        {
+            return;
+        }
+
+        if (BytesLeftToRead > 0)
+        {
+            await _stream.SkipAsync(BytesLeftToRead).ConfigureAwait(false);
+            _amountRead += BytesLeftToRead;
+            BytesLeftToRead = 0;
+        }
+
+        var bytesInLastBlock = _amountRead % 512;
+        if (bytesInLastBlock != 0)
+        {
+            await _stream.SkipAsync(512 - bytesInLastBlock).ConfigureAwait(false);
+        }
+
+        _isPositionedAtNextEntry = true;
+    }
+
     public override bool CanRead => true;
 
     public override bool CanSeek => false;
@@ -104,6 +134,11 @@ internal class TarReadOnlySubStream : Stream
 
     public override int Read(byte[] buffer, int offset, int count)
     {
+        if (BytesLeftToRead <= 0)
+        {
+            AdvanceToNextHeader();
+            return 0;
+        }
         if (BytesLeftToRead < count)
         {
             count = (int)BytesLeftToRead;
@@ -113,6 +148,10 @@ internal class TarReadOnlySubStream : Stream
         {
             BytesLeftToRead -= read;
             _amountRead += read;
+            if (BytesLeftToRead == 0)
+            {
+                AdvanceToNextHeader();
+            }
         }
         return read;
     }
@@ -121,6 +160,7 @@ internal class TarReadOnlySubStream : Stream
     {
         if (BytesLeftToRead <= 0)
         {
+            AdvanceToNextHeader();
             return -1;
         }
         var value = _stream.ReadByte();
@@ -128,6 +168,10 @@ internal class TarReadOnlySubStream : Stream
         {
             --BytesLeftToRead;
             ++_amountRead;
+            if (BytesLeftToRead == 0)
+            {
+                AdvanceToNextHeader();
+            }
         }
         return value;
     }
@@ -139,6 +183,11 @@ internal class TarReadOnlySubStream : Stream
         System.Threading.CancellationToken cancellationToken
     )
     {
+        if (BytesLeftToRead <= 0)
+        {
+            await AdvanceToNextHeaderAsync().ConfigureAwait(false);
+            return 0;
+        }
         if (BytesLeftToRead < count)
         {
             count = (int)BytesLeftToRead;
@@ -150,6 +199,10 @@ internal class TarReadOnlySubStream : Stream
         {
             BytesLeftToRead -= read;
             _amountRead += read;
+            if (BytesLeftToRead == 0)
+            {
+                await AdvanceToNextHeaderAsync().ConfigureAwait(false);
+            }
         }
         return read;
     }
@@ -160,6 +213,11 @@ internal class TarReadOnlySubStream : Stream
         System.Threading.CancellationToken cancellationToken = default
     )
     {
+        if (BytesLeftToRead <= 0)
+        {
+            await AdvanceToNextHeaderAsync().ConfigureAwait(false);
+            return 0;
+        }
         if (BytesLeftToRead < buffer.Length)
         {
             buffer = buffer.Slice(0, (int)BytesLeftToRead);
@@ -169,6 +227,10 @@ internal class TarReadOnlySubStream : Stream
         {
             BytesLeftToRead -= read;
             _amountRead += read;
+            if (BytesLeftToRead == 0)
+            {
+                await AdvanceToNextHeaderAsync().ConfigureAwait(false);
+            }
         }
         return read;
     }

tests/SharpCompress.Test/Tar/TarArchiveAsyncTests.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Text;
@@ -416,4 +417,65 @@ public class TarArchiveAsyncTests : ArchiveTests
         Assert.Equal(5100, localOverrideLink.GroupId);
         Assert.Equal(Convert.ToInt64("777", 8), localOverrideLink.Mode);
     }
+
+    [Fact]
+    public async ValueTask Tar_Read_One_At_A_Time_Without_Disposing_Entry_Stream_Async()
+    {
+        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))
+        {
+            await tarWriter.WriteAsync("file0.txt", testFileStream, null);
+            testFileStream.Position = 0;
+            await tarWriter.WriteAsync("file1.txt", testFileStream, null);
+            tarWriter.WriteDirectory("folder0", null);
+            testFileStream.Position = 0;
+            await tarWriter.WriteAsync("folder0/file_in_folder0.txt", testFileStream, null);
+        }
+
+        memoryStream.Position = 0;
+
+        var entryKeys = new List<string?>();
+        var openEntryStreams = new List<Stream>();
+
+        await using (
+            var archive = await TarArchive.OpenAsyncArchive(
+                new AsyncOnlyStream(memoryStream),
+                ReaderOptions.ForExternalStream
+            )
+        )
+        {
+            await foreach (var entry in archive.EntriesAsync)
+            {
+                entryKeys.Add(entry.Key);
+                if (entry.IsDirectory)
+                {
+                    continue;
+                }
+
+                var tarEntryStream = await entry.OpenEntryStreamAsync();
+                openEntryStreams.Add(tarEntryStream);
+
+                using var testFileStream = new MemoryStream();
+                await tarEntryStream.CopyToAsync(testFileStream);
+                Assert.Equal(testBytes.Length, testFileStream.Length);
+            }
+
+            Assert.Equal(4, await archive.EntriesAsync.CountAsync());
+        }
+
+        openEntryStreams.ForEach(stream => stream.Dispose());
+
+        Assert.Equal(
+            ["file0.txt", "file1.txt", "folder0/", "folder0/file_in_folder0.txt"],
+            entryKeys
+        );
+    }
 }

tests/SharpCompress.Test/Tar/TarArchiveTests.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Text;
@@ -421,6 +422,62 @@ public class TarArchiveTests : ArchiveTests
         Assert.Equal(2, numberOfEntries);
     }
 
+    [Fact]
+    public void Tar_Read_One_At_A_Time_Without_Disposing_Entry_Stream()
+    {
+        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("file0.txt", testFileStream);
+            testFileStream.Position = 0;
+            tarWriter.Write("file1.txt", testFileStream);
+            tarWriter.WriteDirectory("folder0", null);
+            testFileStream.Position = 0;
+            tarWriter.Write("folder0/file_in_folder0.txt", testFileStream);
+        }
+
+        memoryStream.Position = 0;
+
+        var entryKeys = new List<string?>();
+        var openEntryStreams = new List<Stream>();
+
+        using (var archive = ArchiveFactory.OpenArchive(memoryStream))
+        {
+            foreach (var entry in archive.Entries)
+            {
+                entryKeys.Add(entry.Key);
+                if (entry.IsDirectory)
+                {
+                    continue;
+                }
+
+                var tarEntryStream = entry.OpenEntryStream();
+                openEntryStreams.Add(tarEntryStream);
+
+                using var testFileStream = new MemoryStream();
+                tarEntryStream.CopyTo(testFileStream);
+                Assert.Equal(testBytes.Length, testFileStream.Length);
+            }
+
+            Assert.Equal(4, archive.Entries.Count());
+        }
+
+        openEntryStreams.ForEach(stream => stream.Dispose());
+
+        Assert.Equal(
+            ["file0.txt", "file1.txt", "folder0/", "folder0/file_in_folder0.txt"],
+            entryKeys
+        );
+    }
+
     [Fact]
     public void Tar_Detect_Test()
     {

tests/SharpCompress.Test/Tar/TarReaderAsyncTests.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Text;
 using System.Threading.Tasks;
 using SharpCompress.Common;
 using SharpCompress.Common.Tar;
@@ -8,6 +9,7 @@ using SharpCompress.Factories;
 using SharpCompress.Readers;
 using SharpCompress.Readers.Tar;
 using SharpCompress.Test.Mocks;
+using SharpCompress.Writers.Tar;
 using Xunit;
 
 namespace SharpCompress.Test.Tar;
@@ -347,6 +349,62 @@ public class TarReaderAsyncTests : ReaderTests
         );
     }
 
+    [Fact]
+    public async ValueTask Tar_Read_One_At_A_Time_Without_Disposing_Entry_Stream_Async()
+    {
+        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))
+        {
+            await tarWriter.WriteAsync("file0.txt", testFileStream, null);
+            testFileStream.Position = 0;
+            await tarWriter.WriteAsync("file1.txt", testFileStream, null);
+            tarWriter.WriteDirectory("folder0", null);
+            testFileStream.Position = 0;
+            await tarWriter.WriteAsync("folder0/file_in_folder0.txt", testFileStream, null);
+        }
+
+        memoryStream.Position = 0;
+
+        var entryKeys = new List<string?>();
+        var openEntryStreams = new List<Stream>();
+
+        await using (
+            var reader = await TarReader.OpenAsyncReader(new AsyncOnlyStream(memoryStream))
+        )
+        {
+            while (await reader.MoveToNextEntryAsync())
+            {
+                entryKeys.Add(reader.Entry.Key);
+                if (reader.Entry.IsDirectory)
+                {
+                    continue;
+                }
+
+                var entryStream = await reader.OpenEntryStreamAsync();
+                openEntryStreams.Add(entryStream);
+
+                using var testFileStream = new MemoryStream();
+                await entryStream.CopyToAsync(testFileStream);
+                Assert.Equal(testBytes.Length, testFileStream.Length);
+            }
+        }
+
+        openEntryStreams.ForEach(stream => stream.Dispose());
+
+        Assert.Equal(
+            ["file0.txt", "file1.txt", "folder0/", "folder0/file_in_folder0.txt"],
+            entryKeys
+        );
+    }
+
     [Fact]
     public async ValueTask Tar_Corrupted_Async()
     {

tests/SharpCompress.Test/Tar/TarReaderTests.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Text;
 using SharpCompress.Common;
 using SharpCompress.Common.Tar;
 using SharpCompress.Compressors.BZip2;
@@ -8,6 +9,7 @@ using SharpCompress.Factories;
 using SharpCompress.Readers;
 using SharpCompress.Readers.Tar;
 using SharpCompress.Test.Mocks;
+using SharpCompress.Writers.Tar;
 using Xunit;
 
 namespace SharpCompress.Test.Tar;
@@ -379,6 +381,60 @@ public class TarReaderTests : ReaderTests
         Assert.Throws<IncompleteArchiveException>(() => reader.MoveToNextEntry());
     }
 
+    [Fact]
+    public void Tar_Read_One_At_A_Time_Without_Disposing_Entry_Stream()
+    {
+        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("file0.txt", testFileStream, null);
+            testFileStream.Position = 0;
+            tarWriter.Write("file1.txt", testFileStream, null);
+            tarWriter.WriteDirectory("folder0", null);
+            testFileStream.Position = 0;
+            tarWriter.Write("folder0/file_in_folder0.txt", testFileStream, null);
+        }
+
+        memoryStream.Position = 0;
+
+        var entryKeys = new List<string?>();
+        var openEntryStreams = new List<Stream>();
+
+        using (var reader = TarReader.OpenReader(memoryStream))
+        {
+            while (reader.MoveToNextEntry())
+            {
+                entryKeys.Add(reader.Entry.Key);
+                if (reader.Entry.IsDirectory)
+                {
+                    continue;
+                }
+
+                var entryStream = reader.OpenEntryStream();
+                openEntryStreams.Add(entryStream);
+
+                using var testFileStream = new MemoryStream();
+                entryStream.CopyTo(testFileStream);
+                Assert.Equal(testBytes.Length, testFileStream.Length);
+            }
+        }
+
+        openEntryStreams.ForEach(stream => stream.Dispose());
+
+        Assert.Equal(
+            ["file0.txt", "file1.txt", "folder0/", "folder0/file_in_folder0.txt"],
+            entryKeys
+        );
+    }
+
     [Fact]
     public void Tar_Corrupted()
     {