Merge pull request #1372 from adamhathcock/adam/pr-1369

Fixes ZipWriter to be fully forward only

src/SharpCompress/Writers/Zip/ZipCentralDirectoryEntry.cs

@@ -34,6 +34,7 @@ internal class ZipCentralDirectoryEntry
     internal ulong Decompressed { get; set; }
     internal ushort Zip64HeaderOffset { get; set; }
     internal ulong HeaderOffset { get; }
+    internal bool UsesDataDescriptor { get; set; }
 
     internal uint Write(Stream outputStream)
     {
@@ -75,7 +76,7 @@ internal class ZipCentralDirectoryEntry
         var flags = Equals(archiveEncoding.GetEncoding(), Encoding.UTF8)
             ? HeaderFlags.Efs
             : HeaderFlags.None;
-        if (!outputStream.CanSeek)
+        if (UsesDataDescriptor)
         {
             // Cannot use data descriptors with zip64:
             // https://blogs.oracle.com/xuemingshen/entry/is_zipinput_outputstream_handling_of

src/SharpCompress/Writers/Zip/ZipWriter.Async.cs

@@ -118,7 +118,14 @@ public partial class ZipWriter
         }
 
         var headersize = (uint)
-            await WriteHeaderAsync(entryPath, options, entry, useZip64, cancellationToken)
+            await WriteHeaderAsync(
+                    entryPath,
+                    options,
+                    entry,
+                    useZip64,
+                    usesDataDescriptor: !OutputStream.NotNull().CanSeek,
+                    cancellationToken
+                )
                 .ConfigureAwait(false);
         streamPosition += headersize;
         return await ZipWritingStream
@@ -138,16 +145,25 @@ public partial class ZipWriter
         ZipWriterEntryOptions zipWriterEntryOptions,
         ZipCentralDirectoryEntry entry,
         bool useZip64,
+        bool usesDataDescriptor,
         CancellationToken cancellationToken
     )
     {
         // Build the header synchronously into a MemoryStream, then async-copy to OutputStream.
         // This avoids any synchronous writes to the potentially async-only output stream.
         using var ms = new MemoryStream();
-        var result = WriteHeader(ms, filename, zipWriterEntryOptions, entry, useZip64);
+        var outputStream = OutputStream.NotNull();
+        var result = WriteHeader(
+            ms,
+            filename,
+            zipWriterEntryOptions,
+            entry,
+            useZip64,
+            outputStream.CanSeek,
+            usesDataDescriptor
+        );
         ms.Position = 0;
-        await ms.CopyToAsync(OutputStream.NotNull(), 81920, cancellationToken)
-            .ConfigureAwait(false);
+        await ms.CopyToAsync(outputStream, 81920, cancellationToken).ConfigureAwait(false);
         return result;
     }
 
@@ -206,7 +222,14 @@ public partial class ZipWriter
         }
 
         var headersize = (uint)
-            await WriteHeaderAsync(directoryPath, options, entry, useZip64, cancellationToken)
+            await WriteHeaderAsync(
+                    directoryPath,
+                    options,
+                    entry,
+                    useZip64,
+                    usesDataDescriptor: false,
+                    cancellationToken
+                )
                 .ConfigureAwait(false);
         streamPosition += headersize;
         entries.Add(entry);

src/SharpCompress/Writers/Zip/ZipWriter.cs

@@ -117,7 +117,13 @@ public partial class ZipWriter : AbstractWriter
             useZip64 = options.EnableZip64.Value;
         }
 
-        var headersize = (uint)WriteHeader(entryPath, options, entry, useZip64);
+        var headersize = (uint)WriteHeader(
+            entryPath,
+            options,
+            entry,
+            useZip64,
+            usesDataDescriptor: !OutputStream.NotNull().CanSeek
+        );
         streamPosition += headersize;
         return new ZipWritingStream(
             this,
@@ -198,7 +204,13 @@ public partial class ZipWriter : AbstractWriter
             useZip64 = options.EnableZip64.Value;
         }
 
-        var headersize = (uint)WriteHeader(directoryPath, options, entry, useZip64);
+        var headersize = (uint)WriteHeader(
+            directoryPath,
+            options,
+            entry,
+            useZip64,
+            usesDataDescriptor: false
+        );
         streamPosition += headersize;
         entries.Add(entry);
     }
@@ -207,25 +219,42 @@ public partial class ZipWriter : AbstractWriter
         string filename,
         ZipWriterEntryOptions zipWriterEntryOptions,
         ZipCentralDirectoryEntry entry,
-        bool useZip64
-    ) => WriteHeader(OutputStream.NotNull(), filename, zipWriterEntryOptions, entry, useZip64);
+        bool useZip64,
+        bool usesDataDescriptor
+    )
+    {
+        var outputStream = OutputStream.NotNull();
+        return WriteHeader(
+            outputStream,
+            filename,
+            zipWriterEntryOptions,
+            entry,
+            useZip64,
+            outputStream.CanSeek,
+            usesDataDescriptor
+        );
+    }
 
     private int WriteHeader(
         Stream stream,
         string filename,
         ZipWriterEntryOptions zipWriterEntryOptions,
         ZipCentralDirectoryEntry entry,
-        bool useZip64
+        bool useZip64,
+        bool outputCanSeek,
+        bool usesDataDescriptor
     )
     {
         // We err on the side of caution until the zip specification clarifies how to support this
-        if (!stream.CanSeek && useZip64)
+        if (!outputCanSeek && useZip64)
         {
             throw new NotSupportedException(
                 "Zip64 extensions are not supported on non-seekable streams"
             );
         }
 
+        entry.UsesDataDescriptor = usesDataDescriptor;
+
         var explicitZipCompressionInfo = ToZipCompressionMethod(
             zipWriterEntryOptions.CompressionType ?? compressionType
         );
@@ -236,7 +265,7 @@ public partial class ZipWriter : AbstractWriter
         stream.Write(intBuf);
         if (explicitZipCompressionInfo == ZipCompressionMethod.Deflate)
         {
-            if (stream.CanSeek && useZip64)
+            if (outputCanSeek && useZip64)
             {
                 stream.Write(stackalloc byte[] { 45, 0 }); //smallest allowed version for zip64
             }
@@ -252,7 +281,7 @@ public partial class ZipWriter : AbstractWriter
         var flags = Equals(WriterOptions.ArchiveEncoding.GetEncoding(), Encoding.UTF8)
             ? HeaderFlags.Efs
             : 0;
-        if (!stream.CanSeek)
+        if (usesDataDescriptor)
         {
             flags |= HeaderFlags.UsePostDataDescriptor;
 
@@ -280,7 +309,7 @@ public partial class ZipWriter : AbstractWriter
         stream.Write(intBuf.Slice(0, 2)); // filename length
 
         var extralength = 0;
-        if (stream.CanSeek && useZip64)
+        if (outputCanSeek && useZip64)
         {
             extralength = 2 + 2 + 8 + 8;
         }

src/SharpCompress/Writers/Zip/ZipWritingStream.cs

@@ -373,7 +373,7 @@ public partial class ZipWriter
                 var compressedvalue = zip64 ? uint.MaxValue : (uint)countingCount;
                 var decompressedvalue = zip64 ? uint.MaxValue : (uint)entry.Decompressed;
 
-                if (originalStream.CanSeek)
+                if (!entry.UsesDataDescriptor)
                 {
                     originalStream.Position = (long)(entry.HeaderOffset + 6);
                     originalStream.WriteByte(0);
@@ -653,7 +653,7 @@ public partial class ZipWriter
             var compressedvalue = zip64 ? uint.MaxValue : (uint)countingCount;
             var decompressedvalue = zip64 ? uint.MaxValue : (uint)entry.Decompressed;
 
-            if (originalStream.CanSeek)
+            if (!entry.UsesDataDescriptor)
             {
                 originalStream.Position = (long)(entry.HeaderOffset + 6);
                 await originalStream.WriteAsync(new byte[] { 0 }, 0, 1).ConfigureAwait(false);

tests/SharpCompress.Test/GZip/GZipWriterNonSeekableTests.cs

@@ -0,0 +1,73 @@
+using System.IO;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using SharpCompress.Archives.GZip;
+using SharpCompress.Test.Mocks;
+using SharpCompress.Writers.GZip;
+using Xunit;
+
+namespace SharpCompress.Test.GZip;
+
+/// <summary>
+/// Regression tests for writing gzip streams to non-seekable (forward-only) output streams.
+/// GZip is inherently a single forward stream with no seek-dependent layout, so streaming to
+/// a <see cref="ForwardOnlyStream"/> must simply produce a valid, readable gzip stream.
+/// </summary>
+public class GZipWriterNonSeekableTests
+{
+    private const string EntryName = "content.txt";
+
+    private static byte[] CreateContent() =>
+        Encoding.UTF8.GetBytes(string.Concat(Enumerable.Repeat("Hello streaming gzip! ", 500)));
+
+    private static async Task<byte[]> WriteToNonSeekableAsync(byte[] content)
+    {
+        using var ms = new MemoryStream();
+        await using (var writer = new GZipWriter(new ForwardOnlyStream(ms)))
+        {
+            using var source = new MemoryStream(content);
+            await writer.WriteAsync(EntryName, source, null);
+        }
+        return ms.ToArray();
+    }
+
+    private static byte[] WriteToNonSeekableSync(byte[] content)
+    {
+        using var ms = new MemoryStream();
+        using (var writer = new GZipWriter(new ForwardOnlyStream(ms)))
+        {
+            using var source = new MemoryStream(content);
+            writer.Write(EntryName, source, null);
+        }
+        return ms.ToArray();
+    }
+
+    private static void AssertRoundTrips(byte[] gz, byte[] expected)
+    {
+        using var archive = GZipArchive.OpenArchive(new MemoryStream(gz));
+        var entry = archive.Entries.First();
+        using var extracted = new MemoryStream();
+        using (var entryStream = entry.OpenEntryStream())
+        {
+            entryStream.CopyTo(extracted);
+        }
+        Assert.Equal(expected, extracted.ToArray());
+    }
+
+    [Fact]
+    public async Task GZip_Async_NonSeekable_RoundTrips()
+    {
+        var content = CreateContent();
+        var gz = await WriteToNonSeekableAsync(content);
+        AssertRoundTrips(gz, content);
+    }
+
+    [Fact]
+    public void GZip_Sync_NonSeekable_RoundTrips()
+    {
+        var content = CreateContent();
+        var gz = WriteToNonSeekableSync(content);
+        AssertRoundTrips(gz, content);
+    }
+}

tests/SharpCompress.Test/SevenZip/SevenZipWriterNonSeekableTests.cs

@@ -0,0 +1,25 @@
+using System.IO;
+using SharpCompress.Common;
+using SharpCompress.Test.Mocks;
+using SharpCompress.Writers.SevenZip;
+using Xunit;
+
+namespace SharpCompress.Test.SevenZip;
+
+/// <summary>
+/// 7z writing requires a seekable output stream so the signature header can be back-patched on
+/// finalize (see SevenZipWriter.cs). Unlike Zip, it cannot fall back to a streaming layout, so
+/// constructing a writer over a non-seekable output must fail fast. This pins that documented
+/// limitation against silent regression.
+/// </summary>
+public class SevenZipWriterNonSeekableTests
+{
+    [Fact]
+    public void SevenZip_NonSeekable_Output_Throws()
+    {
+        using var ms = new MemoryStream();
+        Assert.Throws<ArchiveOperationException>(() =>
+            new SevenZipWriter(new ForwardOnlyStream(ms), new SevenZipWriterOptions())
+        );
+    }
+}

tests/SharpCompress.Test/Tar/TarWriterNonSeekableTests.cs

@@ -0,0 +1,129 @@
+using System;
+using System.IO;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using SharpCompress.Archives.Tar;
+using SharpCompress.Common;
+using SharpCompress.Test.Mocks;
+using SharpCompress.Writers.Tar;
+using Xunit;
+
+namespace SharpCompress.Test.Tar;
+
+/// <summary>
+/// Regression tests for writing tar archives to non-seekable (forward-only) output streams.
+/// Tar is a forward-only format with no header back-patching, so streaming to a
+/// <see cref="ForwardOnlyStream"/> must produce a valid, readable archive. The sources are
+/// seekable so the writer can derive each entry's size up front (see TarWriter.cs).
+/// </summary>
+public class TarWriterNonSeekableTests
+{
+    private static readonly DateTime FixedModificationTime = new(2024, 5, 15, 10, 30, 0);
+
+    private static (string Name, byte[] Content)[] CreateStreamingTestEntries() =>
+        [
+            (
+                "first.txt",
+                Encoding.UTF8.GetBytes(
+                    string.Concat(Enumerable.Repeat("Hello streaming tar! ", 500))
+                )
+            ),
+            (
+                "nested/second.txt",
+                Encoding.UTF8.GetBytes(
+                    string.Concat(Enumerable.Repeat("Another entry with different content. ", 300))
+                )
+            ),
+        ];
+
+    private static async Task<byte[]> WriteArchiveToNonSeekableAsync(
+        (string Name, byte[] Content)[] entries
+    )
+    {
+        using var ms = new MemoryStream();
+        await using (
+            var writer = new TarWriter(
+                new ForwardOnlyStream(ms),
+                new TarWriterOptions(CompressionType.None, true)
+            )
+        )
+        {
+            foreach (var (name, content) in entries)
+            {
+                using var source = new MemoryStream(content);
+                await writer.WriteAsync(name, source, FixedModificationTime);
+            }
+        }
+        return ms.ToArray();
+    }
+
+    private static byte[] WriteArchiveToNonSeekableSync((string Name, byte[] Content)[] entries)
+    {
+        using var ms = new MemoryStream();
+        using (
+            var writer = new TarWriter(
+                new ForwardOnlyStream(ms),
+                new TarWriterOptions(CompressionType.None, true)
+            )
+        )
+        {
+            foreach (var (name, content) in entries)
+            {
+                using var source = new MemoryStream(content);
+                writer.Write(name, source, FixedModificationTime);
+            }
+        }
+        return ms.ToArray();
+    }
+
+    private static async Task AssertRoundTripsAsync(
+        byte[] tar,
+        (string Name, byte[] Content)[] entries
+    )
+    {
+        using var archive = TarArchive.OpenArchive(new MemoryStream(tar));
+        var fileEntries = archive.Entries.Where(e => !e.IsDirectory).ToList();
+        Assert.Equal(entries.Length, fileEntries.Count);
+        foreach (var (name, content) in entries)
+        {
+            var entry = fileEntries.Single(e => e.Key == name);
+            using var extracted = new MemoryStream();
+#if LEGACY_DOTNET
+            using (var entryStream = await entry.OpenEntryStreamAsync())
+#else
+            await using (var entryStream = await entry.OpenEntryStreamAsync())
+#endif
+            {
+                await entryStream.CopyToAsync(extracted);
+            }
+            Assert.Equal(content, extracted.ToArray());
+        }
+    }
+
+    [Fact]
+    public async Task Tar_Async_NonSeekable_RoundTrips()
+    {
+        var entries = CreateStreamingTestEntries();
+        var tar = await WriteArchiveToNonSeekableAsync(entries);
+        await AssertRoundTripsAsync(tar, entries);
+    }
+
+    [Fact]
+    public async Task Tar_Sync_NonSeekable_RoundTrips()
+    {
+        var entries = CreateStreamingTestEntries();
+        var tar = WriteArchiveToNonSeekableSync(entries);
+        await AssertRoundTripsAsync(tar, entries);
+    }
+
+    [Fact]
+    public async Task Tar_Async_NonSeekable_Matches_Sync_Output()
+    {
+        var entries = CreateStreamingTestEntries();
+        var asyncTar = await WriteArchiveToNonSeekableAsync(entries);
+        var syncTar = WriteArchiveToNonSeekableSync(entries);
+
+        Assert.Equal(syncTar, asyncTar);
+    }
+}

tests/SharpCompress.Test/Zip/ZipWriterNonSeekableTests.cs

@@ -0,0 +1,188 @@
+using System;
+using System.IO;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using SharpCompress.Archives.Zip;
+using SharpCompress.Common;
+using SharpCompress.Test.Mocks;
+using SharpCompress.Writers;
+using SharpCompress.Writers.Zip;
+using Xunit;
+
+namespace SharpCompress.Test.Zip;
+
+/// <summary>
+/// Regression tests for writing zips to non-seekable output streams, where entries must be
+/// written in streaming layout: data-descriptor flag (bit 3) set in the local header and
+/// central directory, zeroed local CRC/sizes, and a trailing PK\x07\x08 descriptor per entry.
+/// The async writer used to derive the flag from its internal buffering MemoryStream instead
+/// of the real output, producing archives that strict readers reject.
+/// </summary>
+public class ZipWriterNonSeekableTests
+{
+    private static readonly DateTime FixedModificationTime = new(2024, 5, 15, 10, 30, 0);
+
+    private static (string Name, byte[] Content)[] CreateStreamingTestEntries() =>
+        [
+            (
+                "first.txt",
+                Encoding.UTF8.GetBytes(
+                    string.Concat(Enumerable.Repeat("Hello streaming zip! ", 500))
+                )
+            ),
+            (
+                "nested/second.txt",
+                Encoding.UTF8.GetBytes(
+                    string.Concat(Enumerable.Repeat("Another entry with different content. ", 300))
+                )
+            ),
+        ];
+
+    private static async Task<byte[]> WriteArchiveToNonSeekableAsync(
+        (string Name, byte[] Content)[] entries
+    )
+    {
+        using var ms = new MemoryStream();
+        await using (
+            var writer = await WriterFactory.OpenAsyncWriter(
+                new ForwardOnlyStream(ms),
+                ArchiveType.Zip,
+                new ZipWriterOptions(CompressionType.Deflate)
+            )
+        )
+        {
+            foreach (var (name, content) in entries)
+            {
+                using var source = new MemoryStream(content);
+                await writer.WriteAsync(name, source, FixedModificationTime);
+            }
+        }
+        return ms.ToArray();
+    }
+
+    private static byte[] WriteArchiveToNonSeekableSync((string Name, byte[] Content)[] entries)
+    {
+        using var ms = new MemoryStream();
+        using (
+            var writer = WriterFactory.OpenWriter(
+                new ForwardOnlyStream(ms),
+                ArchiveType.Zip,
+                new ZipWriterOptions(CompressionType.Deflate)
+            )
+        )
+        {
+            foreach (var (name, content) in entries)
+            {
+                using var source = new MemoryStream(content);
+                writer.Write(name, source, FixedModificationTime);
+            }
+        }
+        return ms.ToArray();
+    }
+
+    private static ushort ReadUInt16(byte[] data, int offset) =>
+        (ushort)(data[offset] | (data[offset + 1] << 8));
+
+    private static uint ReadUInt32(byte[] data, int offset) =>
+        (uint)(
+            data[offset]
+            | (data[offset + 1] << 8)
+            | (data[offset + 2] << 16)
+            | (data[offset + 3] << 24)
+        );
+
+    /// <summary>
+    /// Structurally validates a streamed (non-seekable output) zip: every local header and
+    /// central directory record must have the data-descriptor flag (bit 3, 0x0008) set, local
+    /// CRC/size fields must be zeroed (deferred), and a PK\x07\x08 descriptor with values
+    /// matching the central directory must follow each entry's data.
+    /// </summary>
+    private static void AssertStreamedZipLayout(byte[] zip, int expectedEntries)
+    {
+        // End of central directory record; the archive has no comment, so it is the last 22 bytes
+        var eocd = zip.Length - 22;
+        Assert.Equal(0x06054b50u, ReadUInt32(zip, eocd));
+        int entryCount = ReadUInt16(zip, eocd + 10);
+        Assert.Equal(expectedEntries, entryCount);
+        var cdOffset = checked((int)ReadUInt32(zip, eocd + 16));
+
+        var pos = cdOffset;
+        for (var i = 0; i < entryCount; i++)
+        {
+            Assert.Equal(0x02014b50u, ReadUInt32(zip, pos));
+            var cdFlags = ReadUInt16(zip, pos + 8);
+            Assert.True(
+                (cdFlags & 0x0008) != 0,
+                $"entry {i}: central directory flags 0x{cdFlags:x4} lack the data-descriptor bit"
+            );
+            var crc = ReadUInt32(zip, pos + 16);
+            var compressedSize = ReadUInt32(zip, pos + 20);
+            var uncompressedSize = ReadUInt32(zip, pos + 24);
+            var nameLength = ReadUInt16(zip, pos + 28);
+            var extraLength = ReadUInt16(zip, pos + 30);
+            var commentLength = ReadUInt16(zip, pos + 32);
+            var localHeaderOffset = checked((int)ReadUInt32(zip, pos + 42));
+
+            Assert.Equal(0x04034b50u, ReadUInt32(zip, localHeaderOffset));
+            var localFlags = ReadUInt16(zip, localHeaderOffset + 6);
+            Assert.True(
+                (localFlags & 0x0008) != 0,
+                $"entry {i}: local header flags 0x{localFlags:x4} lack the data-descriptor bit"
+            );
+            Assert.Equal(cdFlags, localFlags);
+            Assert.Equal(0u, ReadUInt32(zip, localHeaderOffset + 14)); // deferred CRC
+            Assert.Equal(0u, ReadUInt32(zip, localHeaderOffset + 18)); // deferred compressed size
+            Assert.Equal(0u, ReadUInt32(zip, localHeaderOffset + 22)); // deferred uncompressed size
+
+            var localNameLength = ReadUInt16(zip, localHeaderOffset + 26);
+            var localExtraLength = ReadUInt16(zip, localHeaderOffset + 28);
+            var descriptor =
+                localHeaderOffset + 30 + localNameLength + localExtraLength + (int)compressedSize;
+            Assert.Equal(0x08074b50u, ReadUInt32(zip, descriptor));
+            Assert.Equal(crc, ReadUInt32(zip, descriptor + 4));
+            Assert.Equal(compressedSize, ReadUInt32(zip, descriptor + 8));
+            Assert.Equal(uncompressedSize, ReadUInt32(zip, descriptor + 12));
+
+            pos += 46 + nameLength + extraLength + commentLength;
+        }
+    }
+
+    [Fact]
+    public async Task Zip_Async_NonSeekable_Writes_DataDescriptor_Flags()
+    {
+        var entries = CreateStreamingTestEntries();
+        var zip = await WriteArchiveToNonSeekableAsync(entries);
+
+        AssertStreamedZipLayout(zip, entries.Length);
+
+        // Round-trip: the archive must be readable and contents intact
+        using var archive = ZipArchive.OpenArchive(new MemoryStream(zip));
+        Assert.Equal(entries.Length, archive.Entries.Count());
+        foreach (var (name, content) in entries)
+        {
+            var entry = archive.Entries.Single(e => e.Key == name);
+            using var extracted = new MemoryStream();
+#if LEGACY_DOTNET
+            using (var entryStream = await entry.OpenEntryStreamAsync())
+#else
+            await using (var entryStream = await entry.OpenEntryStreamAsync())
+#endif
+            {
+                await entryStream.CopyToAsync(extracted);
+            }
+            Assert.Equal(content, extracted.ToArray());
+        }
+    }
+
+    [Fact]
+    public async Task Zip_Async_NonSeekable_Matches_Sync_Output()
+    {
+        var entries = CreateStreamingTestEntries();
+        var asyncZip = await WriteArchiveToNonSeekableAsync(entries);
+        var syncZip = WriteArchiveToNonSeekableSync(entries);
+
+        // The sync writer produces a correct streamed layout; the async writer must match it
+        Assert.Equal(syncZip, asyncZip);
+    }
+}