diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 8962182679..d794c66e27 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -214,7 +214,7 @@ protected override Image Decode(BufferedReadStream stream, Cance break; case PngChunkType.FrameData: { - if (frameCount >= this.maxFrames) + if (frameCount > this.maxFrames) { goto EOF; } @@ -275,7 +275,7 @@ protected override Image Decode(BufferedReadStream stream, Cance previousFrameControl = currentFrameControl; } - if (frameCount >= this.maxFrames) + if (frameCount > this.maxFrames) { goto EOF; } @@ -402,7 +402,7 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok break; case PngChunkType.FrameControl: ++frameCount; - if (frameCount >= this.maxFrames) + if (frameCount > this.maxFrames) { break; } @@ -411,8 +411,12 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok break; case PngChunkType.FrameData: - if (frameCount >= this.maxFrames) + if (frameCount > this.maxFrames) { + // Must skip the chunk data even when we've hit maxFrames, because TryReadChunk + // restores the stream position to the start of the fdAT data after CRC validation. + this.SkipChunkDataAndCrc(chunk); + this.SkipRemainingFrameDataChunks(buffer); break; } @@ -428,9 +432,10 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok InitializeFrameMetadata(framesMetadata, currentFrameControl.Value); - // Skip sequence number - this.currentStream.Skip(4); + // Skip data for this and all remaining FrameData chunks belonging to the same frame + // (comparable to how Decode consumes them via ReadScanlines + ReadNextFrameDataChunk). this.SkipChunkDataAndCrc(chunk); + this.SkipRemainingFrameDataChunks(buffer); break; case PngChunkType.Data: @@ -2093,6 +2098,31 @@ private int ReadNextFrameDataChunk() return 0; } + /// + /// Skips any remaining chunks belonging to the current frame. + /// This mirrors how is used during decoding: + /// consecutive fdAT chunks are consumed until a non-fdAT chunk is encountered, + /// which is stored in for the next iteration. + /// + /// Temporary buffer. + private void SkipRemainingFrameDataChunks(Span buffer) + { + while (this.TryReadChunk(buffer, out PngChunk chunk)) + { + if (chunk.Type is PngChunkType.FrameData) + { + chunk.Data?.Dispose(); + this.SkipChunkDataAndCrc(chunk); + } + else + { + // Not a FrameData chunk; store it so the next TryReadChunk call returns it. + this.nextChunk = chunk; + return; + } + } + } + /// /// Reads a chunk from the stream. /// diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index a58101a6bd..4712fc0dd5 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -411,6 +411,91 @@ public void Identify_IgnoreCrcErrors(string imagePath, int expectedPixelSize) Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel); } + [Fact] + public void Identify_AnimatedPng_ReadsFrameCountCorrectly() + { + TestFile testFile = TestFile.Create(TestImages.Png.AnimatedFrameCount); + + using MemoryStream stream = new(testFile.Bytes, false); + ImageInfo imageInfo = Image.Identify(stream); + + Assert.NotNull(imageInfo); + Assert.Equal(48, imageInfo.FrameMetadataCollection.Count); + } + + [Fact] + public void Identify_AnimatedPngWithMaxFrames_ReadsFrameCountCorrectly() + { + TestFile testFile = TestFile.Create(TestImages.Png.AnimatedFrameCount); + + using MemoryStream stream = new(testFile.Bytes, false); + ImageInfo imageInfo = Image.Identify(new DecoderOptions { MaxFrames = 40 }, stream); + + Assert.NotNull(imageInfo); + Assert.Equal(40, imageInfo.FrameMetadataCollection.Count); + } + + [Fact] + public void Load_AnimatedPngWithMaxFrames_ReadsFrameCountCorrectly() + { + TestFile testFile = TestFile.Create(TestImages.Png.AnimatedFrameCount); + + using MemoryStream stream = new(testFile.Bytes, false); + using Image image = Image.Load(new DecoderOptions { MaxFrames = 40 }, stream); + + Assert.NotNull(image); + Assert.Equal(40, image.Frames.Count); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(5)] + [InlineData(10)] + [InlineData(100)] + public void Identify_AnimatedPng_FrameCount_MatchesDecode(int frameCount) + { + using Image image = new(10, 10, Color.Red.ToPixel()); + for (int i = 1; i < frameCount; i++) + { + using ImageFrame frame = new(Configuration.Default, 10, 10); + image.Frames.AddFrame(frame); + } + + using MemoryStream stream = new(); + image.Save(stream, new PngEncoder()); + stream.Position = 0; + + ImageInfo imageInfo = Image.Identify(stream); + + Assert.NotNull(imageInfo); + Assert.Equal(frameCount, imageInfo.FrameMetadataCollection.Count); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(5)] + [InlineData(10)] + [InlineData(100)] + public void Decode_AnimatedPng_FrameCount(int frameCount) + { + using Image image = new(10, 10, Color.Red.ToPixel()); + for (int i = 1; i < frameCount; i++) + { + using ImageFrame frame = new(Configuration.Default, 10, 10); + image.Frames.AddFrame(frame); + } + + using MemoryStream stream = new(); + image.Save(stream, new PngEncoder()); + stream.Position = 0; + + using Image decoded = Image.Load(stream); + + Assert.Equal(frameCount, decoded.Frames.Count); + } + [Theory] [WithFile(TestImages.Png.Bad.MissingDataChunk, PixelTypes.Rgba32)] public void Decode_MissingDataChunk_ThrowsException(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index fab1b2891c..730e62d824 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -76,6 +76,7 @@ public static class Png public const string BlendOverMultiple = "Png/animated/21-blend-over-multiple.png"; public const string FrameOffset = "Png/animated/frame-offset.png"; public const string DefaultNotAnimated = "Png/animated/default-not-animated.png"; + public const string AnimatedFrameCount = "Png/animated/issue-animated-frame-count.png"; public const string Issue2666 = "Png/issues/Issue_2666.png"; public const string Issue2882 = "Png/issues/Issue_2882.png"; diff --git a/tests/Images/Input/Png/animated/issue-animated-frame-count.png b/tests/Images/Input/Png/animated/issue-animated-frame-count.png new file mode 100644 index 0000000000..88427f4873 --- /dev/null +++ b/tests/Images/Input/Png/animated/issue-animated-frame-count.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:af4e320f586ab26c55612a7ccfc98a8c99cd6a0efe8a70d379503751d06fe8bd +size 51542