diff --git a/src/SharpCompress/Common/Tar/TarFilePart.cs b/src/SharpCompress/Common/Tar/TarFilePart.cs index 15b76d141..eb2f4f337 100644 --- a/src/SharpCompress/Common/Tar/TarFilePart.cs +++ b/src/SharpCompress/Common/Tar/TarFilePart.cs @@ -25,7 +25,7 @@ internal override Stream GetCompressedStream() 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 override Stream GetCompressedStream() { if (_seekableStream != null) { - var useSyncOverAsync = false; -#if LEGACY_DOTNET - useSyncOverAsync = true; -#endif _seekableStream.Position = Header.DataStartPosition ?? 0; - return new ValueTask( - new TarReadOnlySubStream(_seekableStream, Header.Size, useSyncOverAsync) - ); + return new ValueTask(new TarReadOnlySubStream(_seekableStream, Header.Size)); } return new ValueTask(Header.PackedStream.NotNull()); } diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs index 772a56651..c011abd25 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.Async.cs @@ -44,15 +44,7 @@ IArchiveEncoding archiveEncoding 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: diff --git a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs index df71351e9..c95efaef5 100644 --- a/src/SharpCompress/Common/Tar/TarHeaderFactory.cs +++ b/src/SharpCompress/Common/Tar/TarHeaderFactory.cs @@ -38,11 +38,7 @@ IArchiveEncoding archiveEncoding break; case StreamingMode.Streaming: { - header.PackedStream = new TarReadOnlySubStream( - stream, - header.Size, - false - ); + header.PackedStream = new TarReadOnlySubStream(stream, header.Size); } break; default: diff --git a/src/SharpCompress/Common/Tar/TarReadOnlySubStream.cs b/src/SharpCompress/Common/Tar/TarReadOnlySubStream.cs index 987751a34..86594f63d 100644 --- a/src/SharpCompress/Common/Tar/TarReadOnlySubStream.cs +++ b/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 @@ protected override void Dispose(bool disposing) _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,24 +55,62 @@ public override async System.Threading.Tasks.ValueTask DisposeAsync() } _isDisposed = true; - // Ensure we read all remaining blocks for this entry. - await _stream.SkipAsync(BytesLeftToRead).ConfigureAwait(false); - _amountRead += BytesLeftToRead; + await AdvanceToNextHeaderAsync().ConfigureAwait(false); + + GC.SuppressFinalize(this); + await base.DisposeAsync().ConfigureAwait(false); + } +#endif + + private long BytesLeftToRead { get; set; } + + private void AdvanceToNextHeader() + { + if (_isPositionedAtNextEntry) + { + return; + } + + if (BytesLeftToRead > 0) + { + _stream.Skip(BytesLeftToRead); + _amountRead += BytesLeftToRead; + BytesLeftToRead = 0; + } - // If the last block wasn't a full 512 bytes, skip the remaining padding bytes. + // 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); } - GC.SuppressFinalize(this); - await base.DisposeAsync().ConfigureAwait(false); + _isPositionedAtNextEntry = true; } -#endif - - private long BytesLeftToRead { get; set; } public override bool CanRead => true; @@ -104,6 +134,11 @@ public override long Position 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 @@ public override int Read(byte[] buffer, int offset, int count) { BytesLeftToRead -= read; _amountRead += read; + if (BytesLeftToRead == 0) + { + AdvanceToNextHeader(); + } } return read; } @@ -121,6 +160,7 @@ public override int ReadByte() { if (BytesLeftToRead <= 0) { + AdvanceToNextHeader(); return -1; } var value = _stream.ReadByte(); @@ -128,6 +168,10 @@ public override int ReadByte() { --BytesLeftToRead; ++_amountRead; + if (BytesLeftToRead == 0) + { + AdvanceToNextHeader(); + } } return value; } @@ -139,6 +183,11 @@ public override async System.Threading.Tasks.Task ReadAsync( System.Threading.CancellationToken cancellationToken ) { + if (BytesLeftToRead <= 0) + { + await AdvanceToNextHeaderAsync().ConfigureAwait(false); + return 0; + } if (BytesLeftToRead < count) { count = (int)BytesLeftToRead; @@ -150,6 +199,10 @@ System.Threading.CancellationToken cancellationToken { BytesLeftToRead -= read; _amountRead += read; + if (BytesLeftToRead == 0) + { + await AdvanceToNextHeaderAsync().ConfigureAwait(false); + } } return read; } @@ -160,6 +213,11 @@ public override async System.Threading.Tasks.ValueTask ReadAsync( 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 @@ public override async System.Threading.Tasks.ValueTask ReadAsync( { BytesLeftToRead -= read; _amountRead += read; + if (BytesLeftToRead == 0) + { + await AdvanceToNextHeaderAsync().ConfigureAwait(false); + } } return read; } diff --git a/tests/SharpCompress.Test/Tar/TarArchiveAsyncTests.cs b/tests/SharpCompress.Test/Tar/TarArchiveAsyncTests.cs index 1324212f0..28947e97d 100644 --- a/tests/SharpCompress.Test/Tar/TarArchiveAsyncTests.cs +++ b/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 async ValueTask Tar_PaxGlobalHeader_Link_Archive_Async() 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(); + var openEntryStreams = new List(); + + 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 + ); + } } diff --git a/tests/SharpCompress.Test/Tar/TarArchiveTests.cs b/tests/SharpCompress.Test/Tar/TarArchiveTests.cs index 1ea5b3f83..7959967e7 100644 --- a/tests/SharpCompress.Test/Tar/TarArchiveTests.cs +++ b/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 void Tar_Read_One_At_A_Time() 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(); + var openEntryStreams = new List(); + + 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() { diff --git a/tests/SharpCompress.Test/Tar/TarReaderAsyncTests.cs b/tests/SharpCompress.Test/Tar/TarReaderAsyncTests.cs index 5d096c966..d0104cbbe 100644 --- a/tests/SharpCompress.Test/Tar/TarReaderAsyncTests.cs +++ b/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.Readers; using SharpCompress.Readers.Tar; using SharpCompress.Test.Mocks; +using SharpCompress.Writers.Tar; using Xunit; namespace SharpCompress.Test.Tar; @@ -347,6 +349,62 @@ await reader.MoveToNextEntryAsync() ); } + [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(); + var openEntryStreams = new List(); + + 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() { diff --git a/tests/SharpCompress.Test/Tar/TarReaderTests.cs b/tests/SharpCompress.Test/Tar/TarReaderTests.cs index 34a695c89..39c1ba153 100644 --- a/tests/SharpCompress.Test/Tar/TarReaderTests.cs +++ b/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.Readers; using SharpCompress.Readers.Tar; using SharpCompress.Test.Mocks; +using SharpCompress.Writers.Tar; using Xunit; namespace SharpCompress.Test.Tar; @@ -379,6 +381,60 @@ public void Tar_Broken_Stream() Assert.Throws(() => 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(); + var openEntryStreams = new List(); + + 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() {