diff --git a/src/SharpCompress/Compressors/Rar/RarStream.cs b/src/SharpCompress/Compressors/Rar/RarStream.cs index ca718d616..a4869075a 100644 --- a/src/SharpCompress/Compressors/Rar/RarStream.cs +++ b/src/SharpCompress/Compressors/Rar/RarStream.cs @@ -134,7 +134,7 @@ public override int Read(byte[] buffer, int offset, int count) fetch = false; } _position += outTotal; - if (count > 0 && outTotal == 0 && _position != Length) + if (count > 0 && outTotal == 0 && _position < Length) { // sanity check, eg if we try to decompress a redir entry throw new InvalidOperationException( @@ -179,7 +179,7 @@ System.Threading.CancellationToken cancellationToken fetch = false; } _position += outTotal; - if (count > 0 && outTotal == 0 && _position != Length) + if (count > 0 && outTotal == 0 && _position < Length) { // sanity check, eg if we try to decompress a redir entry throw new InvalidOperationException( diff --git a/tests/SharpCompress.Test/Mocks/TruncatedStream.cs b/tests/SharpCompress.Test/Mocks/TruncatedStream.cs new file mode 100644 index 000000000..8699a430d --- /dev/null +++ b/tests/SharpCompress.Test/Mocks/TruncatedStream.cs @@ -0,0 +1,65 @@ +using System; +using System.IO; + +namespace SharpCompress.Test.Mocks; + +/// +/// A stream wrapper that truncates the underlying stream after reading a specified number of bytes. +/// Used for testing error handling when streams end prematurely. +/// +public class TruncatedStream : Stream +{ + private readonly Stream baseStream; + private readonly long truncateAfterBytes; + private long bytesRead; + + public TruncatedStream(Stream baseStream, long truncateAfterBytes) + { + this.baseStream = baseStream ?? throw new ArgumentNullException(nameof(baseStream)); + this.truncateAfterBytes = truncateAfterBytes; + bytesRead = 0; + } + + public override bool CanRead => baseStream.CanRead; + public override bool CanSeek => baseStream.CanSeek; + public override bool CanWrite => false; + public override long Length => baseStream.Length; + + public override long Position + { + get => baseStream.Position; + set => baseStream.Position = value; + } + + public override int Read(byte[] buffer, int offset, int count) + { + if (bytesRead >= truncateAfterBytes) + { + // Simulate premature end of stream + return 0; + } + + var maxBytesToRead = (int)Math.Min(count, truncateAfterBytes - bytesRead); + var actualBytesRead = baseStream.Read(buffer, offset, maxBytesToRead); + bytesRead += actualBytesRead; + return actualBytesRead; + } + + public override long Seek(long offset, SeekOrigin origin) => baseStream.Seek(offset, origin); + + public override void SetLength(long value) => throw new NotSupportedException(); + + public override void Write(byte[] buffer, int offset, int count) => + throw new NotSupportedException(); + + public override void Flush() => baseStream.Flush(); + + protected override void Dispose(bool disposing) + { + if (disposing) + { + baseStream?.Dispose(); + } + base.Dispose(disposing); + } +} diff --git a/tests/SharpCompress.Test/Rar/RarArchiveTests.cs b/tests/SharpCompress.Test/Rar/RarArchiveTests.cs index 991f18e59..ba42f649c 100644 --- a/tests/SharpCompress.Test/Rar/RarArchiveTests.cs +++ b/tests/SharpCompress.Test/Rar/RarArchiveTests.cs @@ -1,3 +1,4 @@ +using System; using System.IO; using System.Linq; using SharpCompress.Archives; @@ -5,6 +6,7 @@ using SharpCompress.Common; using SharpCompress.Compressors.LZMA.Utilites; using SharpCompress.Readers; +using SharpCompress.Test.Mocks; using Xunit; namespace SharpCompress.Test.Rar; @@ -642,4 +644,77 @@ public void Rar_TestEncryptedDetection() ); Assert.True(passwordProtectedFilesArchive.IsEncrypted); } + + /// + /// Test for issue: InvalidOperationException when extracting RAR files. + /// This test verifies the fix for the validation logic that was changed from + /// (_position != Length) to (_position < Length). + /// The old logic would throw an exception when position exceeded expected length, + /// but the new logic only throws when decompression ends prematurely (position < expected). + /// + [Fact] + public void Rar_StreamValidation_OnlyThrowsOnPrematureEnd() + { + // Test normal extraction - should NOT throw InvalidOperationException + // even if actual decompressed size differs from header + var testFiles = new[] { "Rar.rar", "Rar5.rar", "Rar4.rar", "Rar2.rar" }; + + foreach (var testFile in testFiles) + { + using var stream = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, testFile)); + using var archive = RarArchive.Open(stream); + + // Extract all entries and read them completely + foreach (var entry in archive.Entries.Where(e => !e.IsDirectory)) + { + using var entryStream = entry.OpenEntryStream(); + using var ms = new MemoryStream(); + + // This should complete without throwing InvalidOperationException + // The fix ensures we only throw when position < expected length, not when position >= expected + entryStream.CopyTo(ms); + + // Verify we read some data + Assert.True( + ms.Length > 0, + $"Failed to extract data from {entry.Key} in {testFile}" + ); + } + } + } + + /// + /// Negative test case: Verifies that InvalidOperationException IS thrown when + /// a RAR stream ends prematurely (position < expected length). + /// This tests the validation condition (_position < Length) works correctly. + /// + [Fact] + public void Rar_StreamValidation_ThrowsOnTruncatedStream() + { + // This test verifies the exception is thrown when decompression ends prematurely + // by using a truncated stream that stops reading after a small number of bytes + var testFile = "Rar.rar"; + using var fileStream = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, testFile)); + + // Wrap the file stream with a truncated stream that will stop reading early + // This simulates a corrupted or truncated RAR file + using var truncatedStream = new TruncatedStream(fileStream, 1000); + + // Opening the archive should work, but extracting should throw + // when we try to read beyond the truncated data + var exception = Assert.Throws(() => + { + using var archive = RarArchive.Open(truncatedStream); + foreach (var entry in archive.Entries.Where(e => !e.IsDirectory)) + { + using var entryStream = entry.OpenEntryStream(); + using var ms = new MemoryStream(); + // This should throw InvalidOperationException when it can't read all expected bytes + entryStream.CopyTo(ms); + } + }); + + // Verify the exception message matches our expectation + Assert.Contains("unpacked file size does not match header", exception.Message); + } }