Separate Zlib header validation fom stream reading

src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs

@@ -0,0 +1,125 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+using SixLabors.ImageSharp.IO;
+
+namespace SixLabors.ImageSharp.Compression.Zlib;
+
+/// <summary>
+/// A read-only stream over a sequence of length-delimited segments. Bytes are
+/// pulled from the inner stream up to the current segment's remaining length;
+/// when the segment is exhausted the supplied delegate is invoked to advance
+/// to the next segment and return its length. The inner stream is not owned
+/// and is not disposed.
+/// </summary>
+internal sealed class ChunkedReadStream : Stream
+{
+    private static readonly Func<int> GetDataNoOp = () => 0;
+
+    private readonly BufferedReadStream innerStream;
+    private readonly Func<int> getData;
+    private int currentDataRemaining;
+
+    public ChunkedReadStream(BufferedReadStream innerStream)
+        : this(innerStream, GetDataNoOp)
+    {
+    }
+
+    public ChunkedReadStream(BufferedReadStream innerStream, Func<int> getData)
+    {
+        this.innerStream = innerStream;
+        this.getData = getData;
+    }
+
+    /// <inheritdoc/>
+    public override bool CanRead => this.innerStream.CanRead;
+
+    /// <inheritdoc/>
+    public override bool CanSeek => false;
+
+    /// <inheritdoc/>
+    public override bool CanWrite => throw new NotSupportedException();
+
+    /// <inheritdoc/>
+    public override long Length => throw new NotSupportedException();
+
+    /// <inheritdoc/>
+    public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); }
+
+    /// <summary>
+    /// Sets the number of bytes available to read from the current segment.
+    /// Must be called before reading each segment.
+    /// </summary>
+    public void SetCurrentSegmentLength(int bytes) => this.currentDataRemaining = bytes;
+
+    /// <inheritdoc/>
+    public override void Flush() => throw new NotSupportedException();
+
+    /// <inheritdoc/>
+    public override int ReadByte()
+    {
+        this.currentDataRemaining--;
+        return this.innerStream.ReadByte();
+    }
+
+    /// <inheritdoc/>
+    public override int Read(byte[] buffer, int offset, int count)
+    {
+        if (this.currentDataRemaining is 0)
+        {
+            // Current segment is exhausted; ask the caller for the next one.
+            this.currentDataRemaining = this.getData();
+
+            if (this.currentDataRemaining is 0)
+            {
+                return 0;
+            }
+        }
+
+        int bytesToRead = Math.Min(count, this.currentDataRemaining);
+        this.currentDataRemaining -= bytesToRead;
+        int totalBytesRead = this.innerStream.Read(buffer, offset, bytesToRead);
+        long innerStreamLength = this.innerStream.Length;
+
+        // Keep reading data until we've reached the end of the stream or filled the buffer.
+        int bytesRead = 0;
+        offset += totalBytesRead;
+        while (this.currentDataRemaining is 0 && totalBytesRead < count)
+        {
+            this.currentDataRemaining = this.getData();
+
+            if (this.currentDataRemaining is 0)
+            {
+                return totalBytesRead;
+            }
+
+            offset += bytesRead;
+
+            if (offset >= innerStreamLength || offset >= count)
+            {
+                return totalBytesRead;
+            }
+
+            bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining);
+            this.currentDataRemaining -= bytesToRead;
+            bytesRead = this.innerStream.Read(buffer, offset, bytesToRead);
+            if (bytesRead == 0)
+            {
+                return totalBytesRead;
+            }
+
+            totalBytesRead += bytesRead;
+        }
+
+        return totalBytesRead;
+    }
+
+    /// <inheritdoc/>
+    public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
+
+    /// <inheritdoc/>
+    public override void SetLength(long value) => throw new NotSupportedException();
+
+    /// <inheritdoc/>
+    public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();
+}

src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs

@@ -8,9 +8,11 @@ using SixLabors.ImageSharp.IO;
 namespace SixLabors.ImageSharp.Compression.Zlib;
 
 /// <summary>
-/// Provides methods and properties for deframing streams from PNGs.
+/// Reads chunked input, parses the zlib CMF/FLG header, and exposes a
+/// <see cref="DeflateStream"/> over the remaining DEFLATE payload. The
+/// Adler-32 trailer is not validated.
 /// </summary>
-internal sealed class ZlibInflateStream : Stream
+internal sealed class ZlibInflateStream : IDisposable
 {
     /// <summary>
     /// Used to read the Adler-32 and Crc-32 checksums.
@@ -19,94 +21,13 @@ internal sealed class ZlibInflateStream : Stream
     /// </summary>
     private static readonly byte[] ChecksumBuffer = new byte[4];
 
-    /// <summary>
+    private readonly ChunkedReadStream segmentStream;
-    /// A default delegate to get more data from the inner stream.
-    /// </summary>
-    private static readonly Func<int> GetDataNoOp = () => 0;
-
-    /// <summary>
-    /// The inner raw memory stream.
-    /// </summary>
-    private readonly BufferedReadStream innerStream;
-
-    /// <summary>
-    /// A value indicating whether this instance of the given entity has been disposed.
-    /// </summary>
-    /// <value><see langword="true"/> if this instance has been disposed; otherwise, <see langword="false"/>.</value>
-    /// <remarks>
-    /// If the entity is disposed, it must not be disposed a second
-    /// time. The isDisposed field is set the first time the entity
-    /// is disposed. If the isDisposed field is true, then the Dispose()
-    /// method will not dispose again. This help not to prolong the entity's
-    /// life in the Garbage Collector.
-    /// </remarks>
-    private bool isDisposed;
-
-    /// <summary>
-    /// The current data remaining to be read.
-    /// </summary>
-    private int currentDataRemaining;
-
-    /// <summary>
-    /// Delegate to get more data once we've exhausted the current data remaining.
-    /// </summary>
-    private readonly Func<int> getData;
-
-    /// <summary>
-    /// When true, the inflated payload is treated as a raw DEFLATE stream with no zlib
-    /// CMF/FLG header (and no Adler-32 trailer). This is required to decode IDATs in
-    /// Apple's proprietary CgBI PNG variant.
-    /// </summary>
-    private readonly bool noHeader;
 
-    /// <summary>
-    /// Initializes a new instance of the <see cref="ZlibInflateStream"/> class.
-    /// </summary>
-    /// <param name="innerStream">The inner raw stream.</param>
     public ZlibInflateStream(BufferedReadStream innerStream)
-        : this(innerStream, GetDataNoOp, noHeader: false)
+        => this.segmentStream = new ChunkedReadStream(innerStream);
-    {
-    }
 
-    /// <summary>
-    /// Initializes a new instance of the <see cref="ZlibInflateStream"/> class.
-    /// </summary>
-    /// <param name="innerStream">The inner raw stream.</param>
-    /// <param name="getData">A delegate to get more data from the inner stream.</param>
     public ZlibInflateStream(BufferedReadStream innerStream, Func<int> getData)
-        : this(innerStream, getData, noHeader: false)
+        => this.segmentStream = new ChunkedReadStream(innerStream, getData);
-    {
-    }
-
-    /// <summary>
-    /// Initializes a new instance of the <see cref="ZlibInflateStream"/> class.
-    /// </summary>
-    /// <param name="innerStream">The inner raw stream.</param>
-    /// <param name="getData">A delegate to get more data from the inner stream.</param>
-    /// <param name="noHeader">
-    /// When <see langword="true"/>, the payload is treated as raw DEFLATE with no zlib header.
-    /// </param>
-    public ZlibInflateStream(BufferedReadStream innerStream, Func<int> getData, bool noHeader)
-    {
-        this.innerStream = innerStream;
-        this.getData = getData;
-        this.noHeader = noHeader;
-    }
-
-    /// <inheritdoc/>
-    public override bool CanRead => this.innerStream.CanRead;
-
-    /// <inheritdoc/>
-    public override bool CanSeek => false;
-
-    /// <inheritdoc/>
-    public override bool CanWrite => throw new NotSupportedException();
-
-    /// <inheritdoc/>
-    public override long Length => throw new NotSupportedException();
-
-    /// <inheritdoc/>
-    public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); }
 
     /// <summary>
     /// Gets the compressed stream over the deframed inner stream.
@@ -114,15 +35,16 @@ internal sealed class ZlibInflateStream : Stream
     public DeflateStream? CompressedStream { get; private set; }
 
     /// <summary>
-    /// Adds new bytes from a frame found in the original stream.
+    /// Sets the length of the next segment of compressed input and, on first
+    /// call, parses the zlib header.
     /// </summary>
-    /// <param name="bytes">The current remaining data according to the chunk length.</param>
+    /// <param name="bytes">The remaining data length for the current segment.</param>
-    /// <param name="isCriticalChunk">Whether the chunk to be inflated is a critical chunk.</param>
+    /// <param name="isCriticalChunk">Whether to throw on a malformed zlib header.</param>
     /// <returns>The <see cref="bool"/>.</returns>
     [MemberNotNullWhen(true, nameof(CompressedStream))]
     public bool AllocateNewBytes(int bytes, bool isCriticalChunk)
     {
-        this.currentDataRemaining = bytes;
+        this.segmentStream.SetCurrentSegmentLength(bytes);
         if (this.CompressedStream is null)
         {
             return this.InitializeInflateStream(isCriticalChunk);
@@ -131,114 +53,15 @@ internal sealed class ZlibInflateStream : Stream
         return true;
     }
 
-    /// <inheritdoc/>
+    public void Dispose()
-    public override void Flush() => throw new NotSupportedException();
-
-    /// <inheritdoc/>
-    public override int ReadByte()
     {
-        this.currentDataRemaining--;
+        this.CompressedStream?.Dispose();
-        return this.innerStream.ReadByte();
+        this.segmentStream?.Dispose();
-    }
-
-    /// <inheritdoc/>
-    public override int Read(byte[] buffer, int offset, int count)
-    {
-        if (this.currentDataRemaining is 0)
-        {
-            // Last buffer was read in its entirety, let's make sure we don't actually have more in additional IDAT chunks.
-            this.currentDataRemaining = this.getData();
-
-            if (this.currentDataRemaining is 0)
-            {
-                return 0;
-            }
-        }
-
-        int bytesToRead = Math.Min(count, this.currentDataRemaining);
-        this.currentDataRemaining -= bytesToRead;
-        int totalBytesRead = this.innerStream.Read(buffer, offset, bytesToRead);
-        long innerStreamLength = this.innerStream.Length;
-
-        // Keep reading data until we've reached the end of the stream or filled the buffer.
-        int bytesRead = 0;
-        offset += totalBytesRead;
-        while (this.currentDataRemaining is 0 && totalBytesRead < count)
-        {
-            this.currentDataRemaining = this.getData();
-
-            if (this.currentDataRemaining is 0)
-            {
-                return totalBytesRead;
-            }
-
-            offset += bytesRead;
-
-            if (offset >= innerStreamLength || offset >= count)
-            {
-                return totalBytesRead;
-            }
-
-            bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining);
-            this.currentDataRemaining -= bytesToRead;
-            bytesRead = this.innerStream.Read(buffer, offset, bytesToRead);
-            if (bytesRead == 0)
-            {
-                return totalBytesRead;
-            }
-
-            totalBytesRead += bytesRead;
-        }
-
-        return totalBytesRead;
-    }
-
-    /// <inheritdoc/>
-    public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
-
-    /// <inheritdoc/>
-    public override void SetLength(long value) => throw new NotSupportedException();
-
-    /// <inheritdoc/>
-    public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();
-
-    /// <inheritdoc/>
-    protected override void Dispose(bool disposing)
-    {
-        if (this.isDisposed)
-        {
-            return;
-        }
-
-        if (disposing)
-        {
-            // Dispose managed resources.
-            if (this.CompressedStream != null)
-            {
-                this.CompressedStream.Dispose();
-                this.CompressedStream = null;
-            }
-        }
-
-        base.Dispose(disposing);
-
-        // Call the appropriate methods to clean up
-        // unmanaged resources here.
-        // Note disposing is done.
-        this.isDisposed = true;
     }
 
     [MemberNotNullWhen(true, nameof(CompressedStream))]
     private bool InitializeInflateStream(bool isCriticalChunk)
     {
-        // Apple CgBI IDATs omit the zlib CMF/FLG header and the Adler-32 trailer,
-        // wrapping a raw DEFLATE payload directly. Skip the header parsing in that mode.
-        if (this.noHeader)
-        {
-            this.CompressedStream = new DeflateStream(this, CompressionMode.Decompress, true);
-            return true;
-        }
-
         // Read the zlib header : http://tools.ietf.org/html/rfc1950
         // CMF(Compression Method and flags)
         // This byte is divided into a 4 - bit compression method and a
@@ -250,9 +73,8 @@ internal sealed class ZlibInflateStream : Stream
         // +---+---+
         // |CMF|FLG|
         // +---+---+
-        int cmf = this.innerStream.ReadByte();
+        int cmf = this.segmentStream.ReadByte();
-        int flag = this.innerStream.ReadByte();
+        int flag = this.segmentStream.ReadByte();
-        this.currentDataRemaining -= 2;
         if (cmf == -1 || flag == -1)
         {
             return false;
@@ -290,16 +112,13 @@ internal sealed class ZlibInflateStream : Stream
         {
             // We don't need this for inflate so simply skip by the next four bytes.
             // https://tools.ietf.org/html/rfc1950#page-6
-            if (this.innerStream.Read(ChecksumBuffer, 0, 4) != 4)
+            if (this.segmentStream.Read(ChecksumBuffer, 0, 4) != 4)
             {
                 return false;
             }
-
-            this.currentDataRemaining -= 4;
         }
 
-        // Initialize the deflate BufferedReadStream.
+        this.CompressedStream = new DeflateStream(this.segmentStream, CompressionMode.Decompress, leaveOpen: true);
-        this.CompressedStream = new DeflateStream(this, CompressionMode.Decompress, true);
 
         return true;
     }

src/ImageSharp/Formats/Png/PngDecoderCore.cs

@@ -767,7 +767,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore
     /// <param name="chunkLength">The length of the chunk that containing the compressed scanline data.</param>
     /// <param name="image"> The pixel data.</param>
     /// <param name="pngMetadata">The png metadata</param>
-    /// <param name="getData">A delegate to get more data from the inner stream for <see cref="ZlibInflateStream"/>.</param>
+    /// <param name="getData">A delegate to get more data from the inner stream when chunk boundaries are crossed.</param>
     /// <param name="frameControl">The frame control</param>
     /// <param name="cancellationToken">The cancellation token.</param>
     private void ReadScanlines<TPixel>(
@@ -779,14 +779,34 @@ internal sealed class PngDecoderCore : ImageDecoderCore
         CancellationToken cancellationToken)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        using ZlibInflateStream inflateStream = new(this.currentStream, getData, noHeader: this.isCgbi);
+        // CgBI IDATs wrap a raw DEFLATE payload directly (no zlib CMF/FLG header
+        // and no Adler-32 trailer); skip the zlib header parser entirely.
+        if (this.isCgbi)
+        {
+            using ChunkedReadStream segmentStream = new(this.currentStream, getData);
+            segmentStream.SetCurrentSegmentLength(chunkLength);
+            using DeflateStream cgbiDataStream = new(segmentStream, CompressionMode.Decompress, leaveOpen: true);
+            this.DecodeFromDeflate(cgbiDataStream, image, pngMetadata, frameControl, cancellationToken);
+            return;
+        }
+
+        using ZlibInflateStream inflateStream = new(this.currentStream, getData);
         if (!inflateStream.AllocateNewBytes(chunkLength, !this.hasImageData))
         {
             return;
         }
 
-        DeflateStream dataStream = inflateStream.CompressedStream!;
+        this.DecodeFromDeflate(inflateStream.CompressedStream!, image, pngMetadata, frameControl, cancellationToken);
+    }
 
+    private void DecodeFromDeflate<TPixel>(
+        DeflateStream dataStream,
+        ImageFrame<TPixel> image,
+        PngMetadata pngMetadata,
+        in FrameControl frameControl,
+        CancellationToken cancellationToken)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
         if (this.header.InterlaceMethod is PngInterlaceMode.Adam7)
         {
             this.DecodeInterlacedPixelData(frameControl, dataStream, image, pngMetadata, cancellationToken);