Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions src/ImageSharp/Formats/Png/PngDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
break;
case PngChunkType.FrameData:
{
if (frameCount >= this.maxFrames)
if (frameCount > this.maxFrames)
{
goto EOF;
}
Expand Down Expand Up @@ -275,7 +275,7 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
previousFrameControl = currentFrameControl;
}

if (frameCount >= this.maxFrames)
if (frameCount > this.maxFrames)
{
goto EOF;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}

Expand All @@ -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;
Comment on lines +435 to 439
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SkipChunkDataAndCrc(chunk) + SkipRemainingFrameDataChunks(buffer) keeps Identify aligned when processing fdAT, but this logic is bypassed when frameCount >= this.maxFrames (the early break at the top of the FrameData case). Because TryReadChunk() restores the stream position to the start of the fdAT data for FrameData chunks, skipping is still required even when you stop collecting frame metadata; otherwise subsequent chunk reads start mid-payload. Consider ensuring fdAT data is skipped (or exiting Identify) in the maxFrames path too.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreas-eriksson I haven't double checked Copilot here to ensure accuracy but could you please have a look?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the logic a bit to take that into account as well.
The maxFrames seems to have been off-by-one so I changed the logic around that a bit as well.
Added some tests for it.

case PngChunkType.Data:

Expand Down Expand Up @@ -2093,6 +2098,31 @@ private int ReadNextFrameDataChunk()
return 0;
}

/// <summary>
/// Skips any remaining <see cref="PngChunkType.FrameData"/> chunks belonging to the current frame.
/// This mirrors how <see cref="ReadNextFrameDataChunk"/> is used during decoding:
/// consecutive fdAT chunks are consumed until a non-fdAT chunk is encountered,
/// which is stored in <see cref="nextChunk"/> for the next iteration.
/// </summary>
/// <param name="buffer">Temporary buffer.</param>
private void SkipRemainingFrameDataChunks(Span<byte> 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;
}
}
}

/// <summary>
/// Reads a chunk from the stream.
/// </summary>
Expand Down
85 changes: 85 additions & 0 deletions tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Rgba32> image = new(10, 10, Color.Red.ToPixel<Rgba32>());
for (int i = 1; i < frameCount; i++)
{
using ImageFrame<Rgba32> 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<Rgba32> image = new(10, 10, Color.Red.ToPixel<Rgba32>());
for (int i = 1; i < frameCount; i++)
{
using ImageFrame<Rgba32> frame = new(Configuration.Default, 10, 10);
image.Frames.AddFrame(frame);
}

using MemoryStream stream = new();
image.Save(stream, new PngEncoder());
stream.Position = 0;

using Image<Rgba32> decoded = Image.Load<Rgba32>(stream);

Assert.Equal(frameCount, decoded.Frames.Count);
}

[Theory]
[WithFile(TestImages.Png.Bad.MissingDataChunk, PixelTypes.Rgba32)]
public void Decode_MissingDataChunk_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
Expand Down
1 change: 1 addition & 0 deletions tests/ImageSharp.Tests/TestImages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.