From 3c78bec7edbb8f2fd412b4558b87bb4b12e0abf8 Mon Sep 17 00:00:00 2001 From: Jason Finch Date: Sun, 18 Oct 2026 09:11:13 +1000 Subject: [PATCH 1/4] Decode PNG data split by empty chunks, reject short palettes and use big-endian 16-bit tRNS Zero-length IDAT and fdAT chunks are valid, but the data chunk readers returned their length of 0, which the inflater treats as the end of the compressed stream. An empty chunk inside the zlib header, or two in a row, broke decoding. The readers now skip empty chunks. A PLTE chunk shorter than one entry produced an empty color table, which the palette scanline processor indexed outside of. It is now treated as a missing palette, so decoding throws InvalidImageContentException. 16-bit grayscale and truecolor tRNS samples are big-endian, like all PNG multi-byte values, but were read and written little-endian. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 78 +++-- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 9 +- .../Formats/Png/PngDecoderTests.Chunks.cs | 298 ++++++++++++++++++ .../Formats/Png/PngEncoderTests.Chunks.cs | 44 +++ 4 files changed, 394 insertions(+), 35 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 7df41d31ea..bcd46d805f 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1338,7 +1338,9 @@ private void ProcessInterlacedDefilteredScanline( /// The png metadata. private static void AssignColorPalette(ReadOnlySpan palette, ReadOnlySpan alpha, PngMetadata pngMetadata) { - if (palette.Length == 0) + // A palette chunk too short to hold a single entry would produce an empty color table, which the + // scanline processors would index outside of. Treat it as missing so indexed decoding reports it. + if (palette.Length < Unsafe.SizeOf()) { return; } @@ -1380,9 +1382,11 @@ private void AssignTransparentMarkers(ReadOnlySpan alpha, PngMetadata pngM { if (this.header.BitDepth == 16) { - ushort rc = BinaryPrimitives.ReadUInt16LittleEndian(alpha[..2]); - ushort gc = BinaryPrimitives.ReadUInt16LittleEndian(alpha.Slice(2, 2)); - ushort bc = BinaryPrimitives.ReadUInt16LittleEndian(alpha.Slice(4, 2)); + // tRNS samples are stored big-endian like all other PNG multi-byte values, + // matching the byte order used for the 16-bit samples in the image data. + ushort rc = BinaryPrimitives.ReadUInt16BigEndian(alpha[..2]); + ushort gc = BinaryPrimitives.ReadUInt16BigEndian(alpha.Slice(2, 2)); + ushort bc = BinaryPrimitives.ReadUInt16BigEndian(alpha.Slice(4, 2)); pngMetadata.TransparentColor = Color.FromPixel(new Rgb48(rc, gc, bc)); return; @@ -1400,7 +1404,7 @@ private void AssignTransparentMarkers(ReadOnlySpan alpha, PngMetadata pngM { if (this.header.BitDepth == 16) { - pngMetadata.TransparentColor = Color.FromPixel(new L16(BinaryPrimitives.ReadUInt16LittleEndian(alpha[..2]))); + pngMetadata.TransparentColor = Color.FromPixel(new L16(BinaryPrimitives.ReadUInt16BigEndian(alpha[..2]))); } else { @@ -2183,24 +2187,30 @@ private int ReadNextDataChunk() Span buffer = stackalloc byte[20]; - int length = this.currentStream.Read(buffer, 0, 4); - if (length == 0) + // The compressed stream is the concatenation of the payloads of all consecutive data chunks, and the + // specification permits zero-length data chunks. A return value of 0 tells the inflater that the + // compressed stream has ended, so empty chunks are skipped rather than reported. + while (true) { - return 0; - } + // Skip the CRC of the previous chunk. + int length = this.currentStream.Read(buffer, 0, 4); + if (length == 0 || !this.TryReadChunk(buffer, out PngChunk chunk)) + { + return 0; + } - if (this.TryReadChunk(buffer, out PngChunk chunk)) - { - if (chunk.Type is PngChunkType.Data or PngChunkType.FrameData) + if (chunk.Type is not (PngChunkType.Data or PngChunkType.FrameData)) { - chunk.Data?.Dispose(); - return chunk.Length; + this.nextChunk = chunk; + return 0; } - this.nextChunk = chunk; + chunk.Data?.Dispose(); + if (chunk.Length > 0) + { + return chunk.Length; + } } - - return 0; } /// @@ -2216,26 +2226,32 @@ private int ReadNextFrameDataChunk() Span buffer = stackalloc byte[20]; - int length = this.currentStream.Read(buffer, 0, 4); - if (length == 0) - { - return 0; - } - - if (this.TryReadChunk(buffer, out PngChunk chunk)) + // As with data chunks, frame data chunks with no payload after the sequence number are valid and are + // skipped, so that a return value of 0 only ever means that the frame's compressed stream has ended. + while (true) { - if (chunk.Type is PngChunkType.FrameData) + // Skip the CRC of the previous chunk. + int length = this.currentStream.Read(buffer, 0, 4); + if (length == 0 || !this.TryReadChunk(buffer, out PngChunk chunk)) { - chunk.Data?.Dispose(); + return 0; + } - this.currentStream.Position += 4; // Skip sequence number - return chunk.Length - 4; + if (chunk.Type is not PngChunkType.FrameData) + { + this.nextChunk = chunk; + return 0; } - this.nextChunk = chunk; - } + chunk.Data?.Dispose(); - return 0; + this.currentStream.Position += 4; // Skip sequence number + int dataLength = chunk.Length - 4; + if (dataLength > 0) + { + return dataLength; + } + } } /// diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index caf2393d89..13171c8181 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -1247,10 +1247,11 @@ private void WriteTransparencyChunk(Stream stream, PngMetadata pngMetadata) { if (this.use16Bit) { + // tRNS samples are stored big-endian like all other PNG multi-byte values. Rgb48 rgb = pngMetadata.TransparentColor.Value.ToPixel(); - BinaryPrimitives.WriteUInt16LittleEndian(alpha, rgb.R); - BinaryPrimitives.WriteUInt16LittleEndian(alpha.Slice(2, 2), rgb.G); - BinaryPrimitives.WriteUInt16LittleEndian(alpha.Slice(4, 2), rgb.B); + BinaryPrimitives.WriteUInt16BigEndian(alpha, rgb.R); + BinaryPrimitives.WriteUInt16BigEndian(alpha.Slice(2, 2), rgb.G); + BinaryPrimitives.WriteUInt16BigEndian(alpha.Slice(4, 2), rgb.B); this.WriteChunk(stream, PngChunkType.Transparency, this.chunkDataBuffer, 0, 6); } @@ -1269,7 +1270,7 @@ private void WriteTransparencyChunk(Stream stream, PngMetadata pngMetadata) if (this.use16Bit) { L16 l16 = pngMetadata.TransparentColor.Value.ToPixel(); - BinaryPrimitives.WriteUInt16LittleEndian(alpha, l16.PackedValue); + BinaryPrimitives.WriteUInt16BigEndian(alpha, l16.PackedValue); this.WriteChunk(stream, PngChunkType.Transparency, this.chunkDataBuffer, 0, 2); } else diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs index 252ea49603..e99f2ae4be 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs @@ -2,11 +2,13 @@ // Licensed under the Six Labors Split License. using System.Buffers.Binary; +using System.IO.Compression; using System.IO.Hashing; using System.Text; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests.Formats.Png; @@ -291,6 +293,302 @@ public void Decode_tRnsChunk_WithAlphaLengthGreaterColorTableLength_ShouldNotThr using Image image = Image.Load(stream); } + /// + /// The compressed image data may be split across data chunks at any byte, including inside the zlib header, + /// and zero-length data chunks are permitted. Every layout must decode to the same pixels. + /// + /// The name of the data chunk layout to write. + [Theory] + [InlineData("LeadingEmpty")] + [InlineData("ConsecutiveLeadingEmpty")] + [InlineData("FirstOneByte")] + [InlineData("EmptyWithinHeader")] + [InlineData("EveryOneByte")] + [InlineData("EveryOneByteWithEmpty")] + [InlineData("EmptyBetween")] + [InlineData("TrailingEmpty")] + public void Decode_CompressedDataSplitAcrossDataChunks_DecodesPixelExact(string layout) + { + using Image expected = CreateNoiseImage(40, 30, 1); + using MemoryStream stream = new(); + expected.Save(stream, new PngEncoder { ColorType = PngColorType.RgbWithAlpha }); + List<(string Type, byte[] Data)> chunks = ReadChunks(stream.ToArray()); + + byte[] zlib = [.. chunks.Where(c => c.Type == "IDAT").SelectMany(c => c.Data)]; + byte[][] segments = layout switch + { + "LeadingEmpty" => [[], zlib], + "ConsecutiveLeadingEmpty" => [[], [], zlib], + "FirstOneByte" => [zlib[..1], zlib[1..]], + "EmptyWithinHeader" => [zlib[..1], [], zlib[1..]], + "EveryOneByte" => [.. zlib.Select(b => new[] { b })], + "EveryOneByteWithEmpty" => [.. zlib.SelectMany(b => new[] { [b], Array.Empty() })], + "EmptyBetween" => [zlib[..(zlib.Length / 2)], [], zlib[(zlib.Length / 2)..]], + "TrailingEmpty" => [zlib, []], + _ => throw new ArgumentOutOfRangeException(nameof(layout)), + }; + + // Replace the encoder's data chunks with the requested layout, keeping every other chunk in place. + using MemoryStream output = new(); + output.Write(stream.ToArray().AsSpan(0, 8)); + foreach ((string type, byte[] data) in chunks) + { + if (type == "IDAT") + { + continue; + } + + if (type == "IEND") + { + foreach (byte[] segment in segments) + { + WriteChunk(output, "IDAT", segment); + } + } + + WriteChunk(output, type, data); + } + + using Image actual = Image.Load(output.ToArray()); + + ImageComparer.Exact.VerifySimilarity(expected, actual); + } + + /// + /// A frame data chunk may carry nothing but its sequence number. Such chunks must not end the frame's + /// compressed data early, wherever they fall. + /// + [Fact] + public void Decode_EmptyFrameDataChunks_DecodesPixelExact() + { + using Image image = CreateNoiseImage(40, 30, 1); + using (Image frame = CreateNoiseImage(40, 30, 2)) + { + image.Frames.AddFrame(frame.Frames.RootFrame); + } + + using MemoryStream stream = new(); + image.Save(stream, new PngEncoder { ColorType = PngColorType.RgbWithAlpha }); + byte[] encoded = stream.ToArray(); + List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); + Assert.Contains(chunks, c => c.Type == "fdAT"); + + // Split every frame data chunk inside its zlib header, around an empty frame data chunk, and renumber + // the shared fcTL/fdAT sequence so the result stays a well-formed animation. + using MemoryStream output = new(); + output.Write(encoded.AsSpan(0, 8)); + uint sequenceNumber = 0; + foreach ((string type, byte[] data) in chunks) + { + if (type == "fdAT") + { + ReadOnlySpan payload = data.AsSpan(4); + WriteChunk(output, type, [.. GetSequenceNumber(sequenceNumber++), .. payload[..1]]); + WriteChunk(output, type, GetSequenceNumber(sequenceNumber++)); + WriteChunk(output, type, [.. GetSequenceNumber(sequenceNumber++), .. payload[1..]]); + continue; + } + + if (type == "fcTL") + { + BinaryPrimitives.WriteUInt32BigEndian(data, sequenceNumber++); + } + + WriteChunk(output, type, data); + } + + using Image expected = Image.Load(encoded); + using Image actual = Image.Load(output.ToArray()); + + Assert.Equal(expected.Frames.Count, actual.Frames.Count); + for (int i = 0; i < expected.Frames.Count; i++) + { + using Image expectedFrame = expected.Frames.CloneFrame(i); + using Image actualFrame = actual.Frames.CloneFrame(i); + ImageComparer.Exact.VerifySimilarity(expectedFrame, actualFrame); + } + } + + /// + /// A palette chunk too short to hold one entry leaves no colors to index, so it is reported as a missing palette + /// instead of reading outside the color table. + /// + /// The interlace mode of the image. + [Theory] + [InlineData(PngInterlaceMode.None)] + [InlineData(PngInterlaceMode.Adam7)] + public void Decode_PaletteChunkShorterThanOneEntry_ThrowsInvalidImageContentException(PngInterlaceMode interlaceMode) + { + // Zeroed scanlines: filter type None and palette index 0. A 9x9 image has 9 rows of 1 + 9 bytes, or + // 100 bytes across the seven Adam7 passes, each row of which also starts with a filter byte. + byte[] indices = interlaceMode == PngInterlaceMode.Adam7 ? new byte[100] : new byte[90]; + using MemoryStream stream = new(); + stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + WriteChunk(stream, "IHDR", CreateHeaderData(9, 9, 8, PngColorType.Palette, interlaceMode)); + WriteChunk(stream, "PLTE", [10, 20]); + WriteChunk(stream, "IDAT", Compress(indices)); + WriteChunk(stream, "IEND", []); + + Assert.Throws(() => Image.Load(stream.ToArray())); + } + + /// + /// 16-bit grayscale tRNS samples are big-endian, so 0x0102 is transparent and its byte-swapped value is not. + /// + [Fact] + public void Decode_16BitGrayscaleTransparency_IsBigEndian() + { + using MemoryStream stream = new(); + stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + WriteChunk(stream, "IHDR", CreateHeaderData(2, 1, 16, PngColorType.Grayscale, PngInterlaceMode.None)); + WriteChunk(stream, "tRNS", [0x01, 0x02]); + WriteChunk(stream, "IDAT", Compress([0, 0x01, 0x02, 0x02, 0x01])); + WriteChunk(stream, "IEND", []); + byte[] data = stream.ToArray(); + + Assert.Equal(new L16(0x0102), Image.Identify(data).Metadata.GetPngMetadata().TransparentColor?.ToPixel()); + + using Image image = Image.Load(data); + Assert.Equal(ushort.MinValue, image[0, 0].A); + Assert.Equal(ushort.MaxValue, image[1, 0].A); + } + + /// + /// 16-bit truecolor tRNS samples are big-endian, so (0x0102, 0x0304, 0x0506) is transparent and its byte-swapped + /// value is not. + /// + [Fact] + public void Decode_16BitRgbTransparency_IsBigEndian() + { + using MemoryStream stream = new(); + stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); + WriteChunk(stream, "IHDR", CreateHeaderData(2, 1, 16, PngColorType.Rgb, PngInterlaceMode.None)); + WriteChunk(stream, "tRNS", [0x01, 0x02, 0x03, 0x04, 0x05, 0x06]); + WriteChunk(stream, "IDAT", Compress([0, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x02, 0x01, 0x04, 0x03, 0x06, 0x05])); + WriteChunk(stream, "IEND", []); + byte[] data = stream.ToArray(); + + Rgb48 expected = new(0x0102, 0x0304, 0x0506); + Assert.Equal(expected, Image.Identify(data).Metadata.GetPngMetadata().TransparentColor?.ToPixel()); + + using Image image = Image.Load(data); + Assert.Equal(ushort.MinValue, image[0, 0].A); + Assert.Equal(ushort.MaxValue, image[1, 0].A); + } + + /// + /// Creates an image filled with reproducible random colors. + /// + /// The image width. + /// The image height. + /// The random seed. + /// The . + private static Image CreateNoiseImage(int width, int height, int seed) + { + Random random = new(seed); + Image image = new(width, height); + for (int y = 0; y < height; y++) + { + for (int x = 0; x < width; x++) + { + image[x, y] = new Rgba32((byte)random.Next(256), (byte)random.Next(256), (byte)random.Next(256), (byte)random.Next(256)); + } + } + + return image; + } + + /// + /// Splits a PNG file into its chunks without validating them. + /// + /// The PNG file, including the signature. + /// The type and data of each chunk, in file order. + private static List<(string Type, byte[] Data)> ReadChunks(byte[] data) + { + List<(string Type, byte[] Data)> chunks = []; + int offset = 8; + while (offset + 12 <= data.Length) + { + int length = BinaryPrimitives.ReadInt32BigEndian(data.AsSpan(offset)); + chunks.Add((Encoding.ASCII.GetString(data, offset + 4, 4), data.AsSpan(offset + 8, length).ToArray())); + offset += length + 12; + } + + return chunks; + } + + /// + /// Writes a chunk with its length, type, data and a valid CRC. + /// + /// The stream to write to. + /// The four character chunk type. + /// The chunk data. + private static void WriteChunk(Stream stream, string type, ReadOnlySpan data) + { + Span buffer = stackalloc byte[4]; + BinaryPrimitives.WriteInt32BigEndian(buffer, data.Length); + stream.Write(buffer); + + // The CRC covers the chunk type and data but not the length. + byte[] typeBytes = Encoding.ASCII.GetBytes(type); + stream.Write(typeBytes); + stream.Write(data); + + Crc32 crc = new(); + crc.Append(typeBytes); + crc.Append(data); + BinaryPrimitives.WriteUInt32BigEndian(buffer, crc.GetCurrentHashAsUInt32()); + stream.Write(buffer); + } + + /// + /// Creates IHDR chunk data using compression method 0 and filter method 0. + /// + /// The image width. + /// The image height. + /// The bit depth. + /// The color type. + /// The interlace mode. + /// The chunk data. + private static byte[] CreateHeaderData(int width, int height, byte bitDepth, PngColorType colorType, PngInterlaceMode interlaceMode) + { + byte[] data = new byte[13]; + BinaryPrimitives.WriteInt32BigEndian(data, width); + BinaryPrimitives.WriteInt32BigEndian(data.AsSpan(4), height); + data[8] = bitDepth; + data[9] = (byte)colorType; + data[12] = (byte)interlaceMode; + return data; + } + + /// + /// Compresses filtered scanlines into a zlib stream. + /// + /// The filtered scanlines. + /// The zlib stream. + private static byte[] Compress(byte[] scanlines) + { + using MemoryStream output = new(); + using (ZLibStream zlib = new(output, CompressionLevel.Optimal, true)) + { + zlib.Write(scanlines); + } + + return output.ToArray(); + } + + /// + /// Gets the big-endian bytes of an APNG sequence number. + /// + /// The sequence number. + /// The encoded sequence number. + private static byte[] GetSequenceNumber(uint sequenceNumber) + { + byte[] data = new byte[4]; + BinaryPrimitives.WriteUInt32BigEndian(data, sequenceNumber); + return data; + } + private static string GetChunkTypeName(uint value) { byte[] data = new byte[4]; diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs index fdaa70e031..885b12a26c 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs @@ -354,4 +354,48 @@ public void ExcludeFilter_WithNone_DoesNotExcludeChunks() bytesSpan = bytesSpan[(4 + 4 + length + 4)..]; } } + + [Theory] + [InlineData(PngColorType.Grayscale)] + [InlineData(PngColorType.Rgb)] + public void TransparencyChunk_16Bit_IsWrittenBigEndian(PngColorType colorType) + { + // arrange + using Image input = new(2, 1); + PngMetadata metadata = input.Metadata.GetPngMetadata(); + metadata.ColorType = colorType; + metadata.BitDepth = PngBitDepth.Bit16; + metadata.TransparentColor = colorType == PngColorType.Grayscale + ? Color.FromPixel(new L16(0x0102)) + : Color.FromPixel(new Rgb48(0x0102, 0x0304, 0x0506)); + + byte[] expected = colorType == PngColorType.Grayscale + ? [0x01, 0x02] + : [0x01, 0x02, 0x03, 0x04, 0x05, 0x06]; + + using MemoryStream memStream = new(); + + // act + input.Save(memStream, new PngEncoder { ColorType = colorType, BitDepth = PngBitDepth.Bit16 }); + + // assert + byte[] data = memStream.ToArray(); + Span bytesSpan = data.AsSpan(8); // Skip header. + bool transparencyFound = false; + while (bytesSpan.Length > 0) + { + int length = BinaryPrimitives.ReadInt32BigEndian(bytesSpan[..4]); + PngChunkType type = (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(4, 4)); + if (type == PngChunkType.Transparency) + { + transparencyFound = true; + Assert.Equal(expected, bytesSpan.Slice(8, length).ToArray()); + } + + bytesSpan = bytesSpan[(4 + 4 + length + 4)..]; + } + + Assert.True(transparencyFound); + Assert.Equal(metadata.TransparentColor, Image.Identify(data).Metadata.GetPngMetadata().TransparentColor); + } } From c199d51caedde962425c9796e6306ab510b92b3f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 27 Oct 2026 12:37:51 +1000 Subject: [PATCH 2/4] Keep PNG structural validation outside image-data recovery Validate indexed palettes before allocating pixels and reject frame data without a complete sequence number in every integrity mode. Reuse the pending-chunk path to return malformed fdAT chunks from inflation to the outer chunk loop. Keep IDAT payloads separate from frame-data framing. Extend existing chunk tests for CRC policies, empty frame payload layouts, short palettes and missing sequence numbers. Use the existing whole-image comparer instead of cloning every frame. All 463 PNG tests pass in Release on .NET 10. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 51 ++++- .../Formats/Png/PngDecoderTests.Chunks.cs | 214 ++++++++++++++---- .../Formats/Png/PngEncoderTests.Chunks.cs | 4 + 3 files changed, 214 insertions(+), 55 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index bcd46d805f..b54e697e9e 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -216,6 +216,8 @@ protected override Image Decode(BufferedReadStream stream, Cance break; case PngChunkType.FrameData: { + ValidateFrameDataChunk(chunk); + if (frameCount > this.maxFrames) { goto EOF; @@ -258,10 +260,10 @@ protected override Image Decode(BufferedReadStream stream, Cance currentFrameControl ??= new FrameControl((uint)this.header.Width, (uint)this.header.Height); if (image is null) { + // PLTE and tRNS precede IDAT. Validate the required indexed palette before + // allocating pixel storage, outside recovery from compressed-data errors. + this.AssignColorPalette(this.palette, this.paletteAlpha, pngMetadata); this.InitializeImage(metadata, currentFrameControl.Value, out image); - - // Both PLTE and tRNS chunks, if present, have been read at this point as per spec. - AssignColorPalette(this.palette, this.paletteAlpha, pngMetadata); } this.ReadScanlines( @@ -413,6 +415,8 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok break; case PngChunkType.FrameData: + ValidateFrameDataChunk(chunk); + if (frameCount > this.maxFrames) { // Must skip the chunk data even when we've hit maxFrames, because TryReadChunk @@ -454,7 +458,7 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok InitializeFrameMetadata(framesMetadata, currentFrameControl.Value); // Both PLTE and tRNS chunks, if present, have been read at this point as per spec. - AssignColorPalette(this.palette, this.paletteAlpha, pngMetadata); + this.AssignColorPalette(this.palette, this.paletteAlpha, pngMetadata); } this.SkipChunkDataAndCrc(chunk); @@ -1331,17 +1335,22 @@ private void ProcessInterlacedDefilteredScanline( } /// - /// Decodes and assigns the color palette to the metadata + /// Assigns the palette to metadata and rejects indexed images without a complete palette entry. /// /// The palette buffer. /// The alpha palette buffer. /// The png metadata. - private static void AssignColorPalette(ReadOnlySpan palette, ReadOnlySpan alpha, PngMetadata pngMetadata) + private void AssignColorPalette(ReadOnlySpan palette, ReadOnlySpan alpha, PngMetadata pngMetadata) { - // A palette chunk too short to hold a single entry would produce an empty color table, which the - // scanline processors would index outside of. Treat it as missing so indexed decoding reports it. + // Indexed samples require at least one complete RGB entry. Reject a missing table before scanline + // recovery can suppress the error in IgnoreImageData mode. Other color types have an optional palette. if (palette.Length < Unsafe.SizeOf()) { + if (this.pngColorType is PngColorType.Palette) + { + PngThrowHelper.ThrowMissingPalette(); + } + return; } @@ -2199,8 +2208,10 @@ private int ReadNextDataChunk() return 0; } - if (chunk.Type is not (PngChunkType.Data or PngChunkType.FrameData)) + if (chunk.Type is not PngChunkType.Data) { + // Only IDAT payloads belong to this stream. Frame data must return to the outer loop + // for its own framing checks, outside recovery from compressed-data errors. this.nextChunk = chunk; return 0; } @@ -2237,8 +2248,10 @@ private int ReadNextFrameDataChunk() return 0; } - if (chunk.Type is not PngChunkType.FrameData) + if (chunk.Type is not PngChunkType.FrameData || chunk.Length < sizeof(uint)) { + // A short fdAT cannot supply compressed data. Let the outer chunk loop validate it outside + // image-data recovery, just as that loop handles the next frame control or end chunk. this.nextChunk = chunk; return 0; } @@ -2265,20 +2278,34 @@ private void SkipRemainingFrameDataChunks(Span buffer) { while (this.TryReadChunk(buffer, out PngChunk chunk)) { - if (chunk.Type is PngChunkType.FrameData) + if (chunk.Type is PngChunkType.FrameData && chunk.Length >= sizeof(uint)) { chunk.Data?.Dispose(); this.SkipChunkDataAndCrc(chunk); } else { - // Not a FrameData chunk; store it so the next TryReadChunk call returns it. + // Preserve other chunks, including malformed fdAT, for validation by the outer chunk loop. this.nextChunk = chunk; return; } } } + /// + /// Checks the required sequence number before consuming a frame data chunk. + /// + /// The frame data chunk. + private static void ValidateFrameDataChunk(in PngChunk chunk) + { + // A missing sequence number is a structural error in every integrity mode. Call this from the outer + // chunk loop, outside the scope that permits recovery from compressed image-data errors. + if (chunk.Length < sizeof(uint)) + { + PngThrowHelper.ThrowInvalidImageContentException("The frame data chunk does not contain a sequence number."); + } + } + /// /// Reads a chunk from the stream. /// diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs index e99f2ae4be..66f8714f51 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs @@ -298,39 +298,34 @@ public void Decode_tRnsChunk_WithAlphaLengthGreaterColorTableLength_ShouldNotThr /// and zero-length data chunks are permitted. Every layout must decode to the same pixels. /// /// The name of the data chunk layout to write. + /// The segment integrity policy. + /// Whether to corrupt the CRC of each empty data chunk. [Theory] - [InlineData("LeadingEmpty")] - [InlineData("ConsecutiveLeadingEmpty")] - [InlineData("FirstOneByte")] - [InlineData("EmptyWithinHeader")] - [InlineData("EveryOneByte")] - [InlineData("EveryOneByteWithEmpty")] - [InlineData("EmptyBetween")] - [InlineData("TrailingEmpty")] - public void Decode_CompressedDataSplitAcrossDataChunks_DecodesPixelExact(string layout) + [InlineData("LeadingEmpty", SegmentIntegrityHandling.Strict, false)] + [InlineData("ConsecutiveLeadingEmpty", SegmentIntegrityHandling.Strict, false)] + [InlineData("FirstOneByte", SegmentIntegrityHandling.Strict, false)] + [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.Strict, false)] + [InlineData("EveryOneByte", SegmentIntegrityHandling.Strict, false)] + [InlineData("EveryOneByteWithEmpty", SegmentIntegrityHandling.Strict, false)] + [InlineData("EmptyBetween", SegmentIntegrityHandling.Strict, false)] + [InlineData("TrailingEmpty", SegmentIntegrityHandling.Strict, false)] + [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.Strict, true)] + [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.IgnoreAncillary, true)] + [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.IgnoreImageData, true)] + public void Decode_CompressedDataSplitAcrossDataChunks_FollowsIntegrityPolicy(string layout, SegmentIntegrityHandling integrity, bool corruptEmptyChunk) { using Image expected = CreateNoiseImage(40, 30, 1); using MemoryStream stream = new(); expected.Save(stream, new PngEncoder { ColorType = PngColorType.RgbWithAlpha }); - List<(string Type, byte[] Data)> chunks = ReadChunks(stream.ToArray()); + byte[] encoded = stream.ToArray(); + List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); byte[] zlib = [.. chunks.Where(c => c.Type == "IDAT").SelectMany(c => c.Data)]; - byte[][] segments = layout switch - { - "LeadingEmpty" => [[], zlib], - "ConsecutiveLeadingEmpty" => [[], [], zlib], - "FirstOneByte" => [zlib[..1], zlib[1..]], - "EmptyWithinHeader" => [zlib[..1], [], zlib[1..]], - "EveryOneByte" => [.. zlib.Select(b => new[] { b })], - "EveryOneByteWithEmpty" => [.. zlib.SelectMany(b => new[] { [b], Array.Empty() })], - "EmptyBetween" => [zlib[..(zlib.Length / 2)], [], zlib[(zlib.Length / 2)..]], - "TrailingEmpty" => [zlib, []], - _ => throw new ArgumentOutOfRangeException(nameof(layout)), - }; + byte[][] segments = SplitCompressedData(zlib, layout); // Replace the encoder's data chunks with the requested layout, keeping every other chunk in place. using MemoryStream output = new(); - output.Write(stream.ToArray().AsSpan(0, 8)); + output.Write(encoded.AsSpan(0, 8)); foreach ((string type, byte[] data) in chunks) { if (type == "IDAT") @@ -343,13 +338,30 @@ public void Decode_CompressedDataSplitAcrossDataChunks_DecodesPixelExact(string foreach (byte[] segment in segments) { WriteChunk(output, "IDAT", segment); + if (corruptEmptyChunk && segment.Length == 0) + { + // Corrupt only the CRC. The compressed bytes and all chunk boundaries remain valid. + output.GetBuffer()[(int)output.Position - 1] ^= 1; + } } } WriteChunk(output, type, data); } - using Image actual = Image.Load(output.ToArray()); + DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; + if (corruptEmptyChunk && integrity != SegmentIntegrityHandling.IgnoreImageData) + { + InvalidImageContentException exception = Assert.Throws(() => + { + using Image actual = Image.Load(options, output.ToArray()); + }); + + Assert.Equal("CRC Error. PNG IDAT chunk is corrupt!", exception.Message); + return; + } + + using Image actual = Image.Load(options, output.ToArray()); ImageComparer.Exact.VerifySimilarity(expected, actual); } @@ -358,8 +370,21 @@ public void Decode_CompressedDataSplitAcrossDataChunks_DecodesPixelExact(string /// A frame data chunk may carry nothing but its sequence number. Such chunks must not end the frame's /// compressed data early, wherever they fall. /// - [Fact] - public void Decode_EmptyFrameDataChunks_DecodesPixelExact() + /// The frame data layout to write. + /// The segment integrity policy. + /// Whether to corrupt the CRC of each sequence-only frame data chunk. + [Theory] + [InlineData("LeadingEmpty", SegmentIntegrityHandling.Strict, false)] + [InlineData("ConsecutiveLeadingEmpty", SegmentIntegrityHandling.Strict, false)] + [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.Strict, false)] + [InlineData("TrailingEmpty", SegmentIntegrityHandling.Strict, false)] + [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.Strict, true)] + [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.IgnoreAncillary, true)] + [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.IgnoreImageData, true)] + [InlineData("TruncatedSequenceBetween", SegmentIntegrityHandling.Strict, false)] + [InlineData("TruncatedSequenceBetween", SegmentIntegrityHandling.IgnoreAncillary, false)] + [InlineData("TruncatedSequenceBetween", SegmentIntegrityHandling.IgnoreImageData, false)] + public void Decode_EmptyFrameDataChunks_FollowsIntegrityPolicy(string layout, SegmentIntegrityHandling integrity, bool corruptEmptyChunk) { using Image image = CreateNoiseImage(40, 30, 1); using (Image frame = CreateNoiseImage(40, 30, 2)) @@ -373,8 +398,7 @@ public void Decode_EmptyFrameDataChunks_DecodesPixelExact() List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); Assert.Contains(chunks, c => c.Type == "fdAT"); - // Split every frame data chunk inside its zlib header, around an empty frame data chunk, and renumber - // the shared fcTL/fdAT sequence so the result stays a well-formed animation. + // Renumber fcTL and fdAT together so rechunking preserves the animation's sequence order. using MemoryStream output = new(); output.Write(encoded.AsSpan(0, 8)); uint sequenceNumber = 0; @@ -382,10 +406,26 @@ public void Decode_EmptyFrameDataChunks_DecodesPixelExact() { if (type == "fdAT") { - ReadOnlySpan payload = data.AsSpan(4); - WriteChunk(output, type, [.. GetSequenceNumber(sequenceNumber++), .. payload[..1]]); - WriteChunk(output, type, GetSequenceNumber(sequenceNumber++)); - WriteChunk(output, type, [.. GetSequenceNumber(sequenceNumber++), .. payload[1..]]); + foreach (byte[] segment in SplitCompressedData(data[4..], layout)) + { + byte[] sequence = GetSequenceNumber(sequenceNumber++); + if (layout == "TruncatedSequenceBetween" && segment.Length == 0) + { + // The preceding payload reaches the inflater before this structural error is encountered. + WriteChunk(output, type, sequence.AsSpan(0, 3)); + } + else + { + WriteChunk(output, type, [.. sequence, .. segment]); + } + + if (corruptEmptyChunk && segment.Length == 0) + { + // fdAT is image data even though its chunk type is ancillary in the PNG naming scheme. + output.GetBuffer()[(int)output.Position - 1] ^= 1; + } + } + continue; } @@ -398,15 +438,68 @@ public void Decode_EmptyFrameDataChunks_DecodesPixelExact() } using Image expected = Image.Load(encoded); - using Image actual = Image.Load(output.ToArray()); + DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; + if (layout == "TruncatedSequenceBetween") + { + Assert.Throws(() => + { + using Image actual = Image.Load(options, output.ToArray()); + }); + + Assert.Throws(() => Image.Identify(options, output.ToArray())); + return; + } + + if (corruptEmptyChunk && integrity != SegmentIntegrityHandling.IgnoreImageData) + { + InvalidImageContentException exception = Assert.Throws(() => + { + using Image actual = Image.Load(options, output.ToArray()); + }); - Assert.Equal(expected.Frames.Count, actual.Frames.Count); - for (int i = 0; i < expected.Frames.Count; i++) + Assert.Equal("CRC Error. PNG fdAT chunk is corrupt!", exception.Message); + return; + } + + using Image actual = Image.Load(options, output.ToArray()); + ImageComparer.Exact.VerifySimilarity(expected, actual); + } + + /// + /// Rejects frame data that cannot contain a sequence number, regardless of the recoverable-error policy. + /// + /// The invalid frame data length. + /// The segment integrity policy. + [Theory] + [InlineData(0, SegmentIntegrityHandling.Strict)] + [InlineData(3, SegmentIntegrityHandling.Strict)] + [InlineData(0, SegmentIntegrityHandling.IgnoreAncillary)] + [InlineData(3, SegmentIntegrityHandling.IgnoreAncillary)] + [InlineData(0, SegmentIntegrityHandling.IgnoreImageData)] + [InlineData(3, SegmentIntegrityHandling.IgnoreImageData)] + public void DecodeAndIdentify_FrameDataWithoutSequenceNumber_ThrowsInvalidImageContentException(int length, SegmentIntegrityHandling integrity) + { + byte[] encoded = TestFile.Create(TestImages.Png.APng).Bytes; + List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); + Assert.Contains(chunks, chunk => chunk.Type == "fdAT"); + + using MemoryStream stream = new(); + stream.Write(encoded.AsSpan(0, 8)); + foreach ((string type, byte[] data) in chunks) { - using Image expectedFrame = expected.Frames.CloneFrame(i); - using Image actualFrame = actual.Frames.CloneFrame(i); - ImageComparer.Exact.VerifySimilarity(expectedFrame, actualFrame); + // Preserve valid CRCs so the failure must come from the missing structural field. + WriteChunk(stream, type, type == "fdAT" ? data.AsSpan(0, length) : data); } + + byte[] invalid = stream.ToArray(); + DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; + InvalidImageContentException exception = Assert.Throws(() => + { + using Image actual = Image.Load(options, invalid); + }); + + Assert.Equal("The frame data chunk does not contain a sequence number.", exception.Message); + Assert.Throws(() => Image.Identify(options, invalid)); } /// @@ -414,10 +507,21 @@ public void Decode_EmptyFrameDataChunks_DecodesPixelExact() /// instead of reading outside the color table. /// /// The interlace mode of the image. + /// The number of palette bytes, fewer than one complete entry. + /// The segment integrity policy. [Theory] - [InlineData(PngInterlaceMode.None)] - [InlineData(PngInterlaceMode.Adam7)] - public void Decode_PaletteChunkShorterThanOneEntry_ThrowsInvalidImageContentException(PngInterlaceMode interlaceMode) + [InlineData(PngInterlaceMode.None, 0, SegmentIntegrityHandling.Strict)] + [InlineData(PngInterlaceMode.None, 1, SegmentIntegrityHandling.Strict)] + [InlineData(PngInterlaceMode.None, 2, SegmentIntegrityHandling.Strict)] + [InlineData(PngInterlaceMode.Adam7, 0, SegmentIntegrityHandling.Strict)] + [InlineData(PngInterlaceMode.Adam7, 1, SegmentIntegrityHandling.Strict)] + [InlineData(PngInterlaceMode.Adam7, 2, SegmentIntegrityHandling.Strict)] + [InlineData(PngInterlaceMode.None, 2, SegmentIntegrityHandling.IgnoreAncillary)] + [InlineData(PngInterlaceMode.None, 2, SegmentIntegrityHandling.IgnoreImageData)] + public void Decode_PaletteChunkShorterThanOneEntry_ThrowsInvalidImageContentException( + PngInterlaceMode interlaceMode, + int paletteLength, + SegmentIntegrityHandling integrity) { // Zeroed scanlines: filter type None and palette index 0. A 9x9 image has 9 rows of 1 + 9 bytes, or // 100 bytes across the seven Adam7 passes, each row of which also starts with a filter byte. @@ -425,11 +529,15 @@ public void Decode_PaletteChunkShorterThanOneEntry_ThrowsInvalidImageContentExce using MemoryStream stream = new(); stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); WriteChunk(stream, "IHDR", CreateHeaderData(9, 9, 8, PngColorType.Palette, interlaceMode)); - WriteChunk(stream, "PLTE", [10, 20]); + WriteChunk(stream, "PLTE", new byte[paletteLength]); WriteChunk(stream, "IDAT", Compress(indices)); WriteChunk(stream, "IEND", []); - Assert.Throws(() => Image.Load(stream.ToArray())); + DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; + Assert.Throws(() => + { + using Image actual = Image.Load(options, stream.ToArray()); + }); } /// @@ -476,6 +584,26 @@ public void Decode_16BitRgbTransparency_IsBigEndian() Assert.Equal(ushort.MaxValue, image[1, 0].A); } + /// + /// Splits compressed data without changing its bytes, including layouts with empty chunks. + /// + /// The complete compressed data. + /// The chunk layout. + /// The ordered chunk payloads. + private static byte[][] SplitCompressedData(byte[] data, string layout) + => layout switch + { + "LeadingEmpty" => [[], data], + "ConsecutiveLeadingEmpty" => [[], [], data], + "FirstOneByte" => [data[..1], data[1..]], + "EmptyWithinHeader" => [data[..1], [], data[1..]], + "EveryOneByte" => [.. data.Select(b => new[] { b })], + "EveryOneByteWithEmpty" => [.. data.SelectMany(b => new[] { [b], Array.Empty() })], + "EmptyBetween" or "TruncatedSequenceBetween" => [data[..(data.Length / 2)], [], data[(data.Length / 2)..]], + "TrailingEmpty" => [data, []], + _ => throw new ArgumentOutOfRangeException(nameof(layout)), + }; + /// /// Creates an image filled with reproducible random colors. /// diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs index 885b12a26c..f1bb985e55 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs @@ -355,6 +355,10 @@ public void ExcludeFilter_WithNone_DoesNotExcludeChunks() } } + /// + /// Checks the stored byte order independently of the decoder, then checks the identified transparency marker. + /// + /// The PNG color type. [Theory] [InlineData(PngColorType.Grayscale)] [InlineData(PngColorType.Rgb)] From 1fea337ceff5e1606faad60e5ae1e26019889e89 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 27 Oct 2026 12:46:04 +1000 Subject: [PATCH 3/4] Reuse stored PNG inputs in chunk regressions Load a stored noise PNG and the existing APNG through TestFile instead of generating random pixels and encoding baseline images for every case. Remove the image generator and defer baseline decoding until a pixel comparison is required. --- .../Formats/Png/PngDecoderTests.Chunks.cs | 40 ++----------------- tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Png/noise-rgba32.png | 3 ++ 3 files changed, 8 insertions(+), 36 deletions(-) create mode 100644 tests/Images/Input/Png/noise-rgba32.png diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs index 66f8714f51..80a072e072 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs @@ -314,10 +314,7 @@ public void Decode_tRnsChunk_WithAlphaLengthGreaterColorTableLength_ShouldNotThr [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.IgnoreImageData, true)] public void Decode_CompressedDataSplitAcrossDataChunks_FollowsIntegrityPolicy(string layout, SegmentIntegrityHandling integrity, bool corruptEmptyChunk) { - using Image expected = CreateNoiseImage(40, 30, 1); - using MemoryStream stream = new(); - expected.Save(stream, new PngEncoder { ColorType = PngColorType.RgbWithAlpha }); - byte[] encoded = stream.ToArray(); + byte[] encoded = TestFile.Create(TestImages.Png.Noise).Bytes; List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); byte[] zlib = [.. chunks.Where(c => c.Type == "IDAT").SelectMany(c => c.Data)]; @@ -361,6 +358,7 @@ public void Decode_CompressedDataSplitAcrossDataChunks_FollowsIntegrityPolicy(st return; } + using Image expected = Image.Load(encoded); using Image actual = Image.Load(options, output.ToArray()); ImageComparer.Exact.VerifySimilarity(expected, actual); @@ -386,15 +384,7 @@ public void Decode_CompressedDataSplitAcrossDataChunks_FollowsIntegrityPolicy(st [InlineData("TruncatedSequenceBetween", SegmentIntegrityHandling.IgnoreImageData, false)] public void Decode_EmptyFrameDataChunks_FollowsIntegrityPolicy(string layout, SegmentIntegrityHandling integrity, bool corruptEmptyChunk) { - using Image image = CreateNoiseImage(40, 30, 1); - using (Image frame = CreateNoiseImage(40, 30, 2)) - { - image.Frames.AddFrame(frame.Frames.RootFrame); - } - - using MemoryStream stream = new(); - image.Save(stream, new PngEncoder { ColorType = PngColorType.RgbWithAlpha }); - byte[] encoded = stream.ToArray(); + byte[] encoded = TestFile.Create(TestImages.Png.APng).Bytes; List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); Assert.Contains(chunks, c => c.Type == "fdAT"); @@ -437,7 +427,6 @@ public void Decode_EmptyFrameDataChunks_FollowsIntegrityPolicy(string layout, Se WriteChunk(output, type, data); } - using Image expected = Image.Load(encoded); DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; if (layout == "TruncatedSequenceBetween") { @@ -461,6 +450,7 @@ public void Decode_EmptyFrameDataChunks_FollowsIntegrityPolicy(string layout, Se return; } + using Image expected = Image.Load(encoded); using Image actual = Image.Load(options, output.ToArray()); ImageComparer.Exact.VerifySimilarity(expected, actual); } @@ -604,28 +594,6 @@ private static byte[][] SplitCompressedData(byte[] data, string layout) _ => throw new ArgumentOutOfRangeException(nameof(layout)), }; - /// - /// Creates an image filled with reproducible random colors. - /// - /// The image width. - /// The image height. - /// The random seed. - /// The . - private static Image CreateNoiseImage(int width, int height, int seed) - { - Random random = new(seed); - Image image = new(width, height); - for (int y = 0; y < height; y++) - { - for (int x = 0; x < width; x++) - { - image[x, y] = new Rgba32((byte)random.Next(256), (byte)random.Next(256), (byte)random.Next(256), (byte)random.Next(256)); - } - } - - return image; - } - /// /// Splits a PNG file into its chunks without validating them. /// diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 24e1bf83fc..f070717d70 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -55,6 +55,7 @@ public static class Png public const string Gray4BitTrans = "Png/gray-4-tRNS.png"; public const string L8BitTrans = "Png/gray-8-tRNS.png"; public const string LowColorVariance = "Png/low-variance.png"; + public const string Noise = "Png/noise-rgba32.png"; public const string PngWithMetadata = "Png/PngWithMetaData.png"; public const string InvalidTextData = "Png/InvalidTextData.png"; public const string DuplicateHeaderChunkResync = "Png/duplicate-header-chunk-resync.png"; diff --git a/tests/Images/Input/Png/noise-rgba32.png b/tests/Images/Input/Png/noise-rgba32.png new file mode 100644 index 0000000000..ef275bd4dd --- /dev/null +++ b/tests/Images/Input/Png/noise-rgba32.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:269ce0e5665d956a332b494c1c4fc5cbeceef4c59033368e7c614f543fc39d18 +size 4898 From 2320d6ef4ee1b0ef6efe9adfac61b45b7cf96431 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 27 Oct 2026 13:06:50 +1000 Subject: [PATCH 4/4] Replace PNG test input builders with fixed files Store all chunk layouts, corrupt-CRC variants, short palettes, truncated frame chunks and asymmetric 16-bit transparency samples as input files. Tests load those files without constructing or compressing PNG input. Encoder tests also load their input images from files. Remove six input-building helpers. Independently check every fixture for chunk bounds, CRC classification and payload contents; Pillow verifies unchanged pixels for all twelve valid rechunked inputs. All 463 PNG tests pass in Release on .NET 10. --- .../Formats/Png/PngDecoderTests.Chunks.cs | 375 ++++-------------- .../Formats/Png/PngEncoderTests.Chunks.cs | 9 +- tests/ImageSharp.Tests/TestImages.cs | 27 ++ .../chunks/fdat-consecutive-leading-empty.png | 3 + .../Input/Png/chunks/fdat-empty-bad-crc.png | 3 + .../Png/chunks/fdat-empty-within-header.png | 3 + .../Input/Png/chunks/fdat-leading-empty.png | 3 + .../Images/Input/Png/chunks/fdat-short-0.png | 3 + .../Images/Input/Png/chunks/fdat-short-3.png | 3 + .../Input/Png/chunks/fdat-trailing-empty.png | 3 + .../fdat-truncated-sequence-between.png | 3 + .../chunks/idat-consecutive-leading-empty.png | 3 + .../Input/Png/chunks/idat-empty-bad-crc.png | 3 + .../Input/Png/chunks/idat-empty-between.png | 3 + .../Png/chunks/idat-empty-within-header.png | 3 + .../chunks/idat-every-one-byte-with-empty.png | 3 + .../Input/Png/chunks/idat-every-one-byte.png | 3 + .../Input/Png/chunks/idat-first-one-byte.png | 3 + .../Input/Png/chunks/idat-leading-empty.png | 3 + .../Input/Png/chunks/idat-trailing-empty.png | 3 + .../Png/chunks/palette-adam7-short-0.png | 3 + .../Png/chunks/palette-adam7-short-1.png | 3 + .../Png/chunks/palette-adam7-short-2.png | 3 + .../Input/Png/chunks/palette-short-0.png | 3 + .../Input/Png/chunks/palette-short-1.png | 3 + .../Input/Png/chunks/palette-short-2.png | 3 + .../Png/chunks/trns-gray16-big-endian.png | 3 + .../Png/chunks/trns-rgb16-big-endian.png | 3 + 28 files changed, 174 insertions(+), 312 deletions(-) create mode 100644 tests/Images/Input/Png/chunks/fdat-consecutive-leading-empty.png create mode 100644 tests/Images/Input/Png/chunks/fdat-empty-bad-crc.png create mode 100644 tests/Images/Input/Png/chunks/fdat-empty-within-header.png create mode 100644 tests/Images/Input/Png/chunks/fdat-leading-empty.png create mode 100644 tests/Images/Input/Png/chunks/fdat-short-0.png create mode 100644 tests/Images/Input/Png/chunks/fdat-short-3.png create mode 100644 tests/Images/Input/Png/chunks/fdat-trailing-empty.png create mode 100644 tests/Images/Input/Png/chunks/fdat-truncated-sequence-between.png create mode 100644 tests/Images/Input/Png/chunks/idat-consecutive-leading-empty.png create mode 100644 tests/Images/Input/Png/chunks/idat-empty-bad-crc.png create mode 100644 tests/Images/Input/Png/chunks/idat-empty-between.png create mode 100644 tests/Images/Input/Png/chunks/idat-empty-within-header.png create mode 100644 tests/Images/Input/Png/chunks/idat-every-one-byte-with-empty.png create mode 100644 tests/Images/Input/Png/chunks/idat-every-one-byte.png create mode 100644 tests/Images/Input/Png/chunks/idat-first-one-byte.png create mode 100644 tests/Images/Input/Png/chunks/idat-leading-empty.png create mode 100644 tests/Images/Input/Png/chunks/idat-trailing-empty.png create mode 100644 tests/Images/Input/Png/chunks/palette-adam7-short-0.png create mode 100644 tests/Images/Input/Png/chunks/palette-adam7-short-1.png create mode 100644 tests/Images/Input/Png/chunks/palette-adam7-short-2.png create mode 100644 tests/Images/Input/Png/chunks/palette-short-0.png create mode 100644 tests/Images/Input/Png/chunks/palette-short-1.png create mode 100644 tests/Images/Input/Png/chunks/palette-short-2.png create mode 100644 tests/Images/Input/Png/chunks/trns-gray16-big-endian.png create mode 100644 tests/Images/Input/Png/chunks/trns-rgb16-big-endian.png diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs index 80a072e072..cc3aa8664e 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. using System.Buffers.Binary; -using System.IO.Compression; using System.IO.Hashing; using System.Text; using SixLabors.ImageSharp.Formats; @@ -294,256 +293,135 @@ public void Decode_tRnsChunk_WithAlphaLengthGreaterColorTableLength_ShouldNotThr } /// - /// The compressed image data may be split across data chunks at any byte, including inside the zlib header, - /// and zero-length data chunks are permitted. Every layout must decode to the same pixels. + /// Checks stored IDAT layouts and applies the configured policy to the input's CRC errors. /// - /// The name of the data chunk layout to write. + /// The stored PNG input. /// The segment integrity policy. - /// Whether to corrupt the CRC of each empty data chunk. + /// Whether the input contains an invalid image-data CRC. [Theory] - [InlineData("LeadingEmpty", SegmentIntegrityHandling.Strict, false)] - [InlineData("ConsecutiveLeadingEmpty", SegmentIntegrityHandling.Strict, false)] - [InlineData("FirstOneByte", SegmentIntegrityHandling.Strict, false)] - [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.Strict, false)] - [InlineData("EveryOneByte", SegmentIntegrityHandling.Strict, false)] - [InlineData("EveryOneByteWithEmpty", SegmentIntegrityHandling.Strict, false)] - [InlineData("EmptyBetween", SegmentIntegrityHandling.Strict, false)] - [InlineData("TrailingEmpty", SegmentIntegrityHandling.Strict, false)] - [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.Strict, true)] - [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.IgnoreAncillary, true)] - [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.IgnoreImageData, true)] - public void Decode_CompressedDataSplitAcrossDataChunks_FollowsIntegrityPolicy(string layout, SegmentIntegrityHandling integrity, bool corruptEmptyChunk) + [InlineData(TestImages.Png.IdatLeadingEmpty, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.IdatConsecutiveLeadingEmpty, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.IdatFirstOneByte, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.IdatEmptyWithinHeader, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.IdatEveryOneByte, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.IdatEveryOneByteWithEmpty, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.IdatEmptyBetween, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.IdatTrailingEmpty, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.IdatEmptyBadCrc, SegmentIntegrityHandling.Strict, true)] + [InlineData(TestImages.Png.IdatEmptyBadCrc, SegmentIntegrityHandling.IgnoreAncillary, true)] + [InlineData(TestImages.Png.IdatEmptyBadCrc, SegmentIntegrityHandling.IgnoreImageData, true)] + public void Decode_CompressedDataSplitAcrossDataChunks_FollowsIntegrityPolicy(string path, SegmentIntegrityHandling integrity, bool hasInvalidCrc) { - byte[] encoded = TestFile.Create(TestImages.Png.Noise).Bytes; - List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); - - byte[] zlib = [.. chunks.Where(c => c.Type == "IDAT").SelectMany(c => c.Data)]; - byte[][] segments = SplitCompressedData(zlib, layout); - - // Replace the encoder's data chunks with the requested layout, keeping every other chunk in place. - using MemoryStream output = new(); - output.Write(encoded.AsSpan(0, 8)); - foreach ((string type, byte[] data) in chunks) - { - if (type == "IDAT") - { - continue; - } - - if (type == "IEND") - { - foreach (byte[] segment in segments) - { - WriteChunk(output, "IDAT", segment); - if (corruptEmptyChunk && segment.Length == 0) - { - // Corrupt only the CRC. The compressed bytes and all chunk boundaries remain valid. - output.GetBuffer()[(int)output.Position - 1] ^= 1; - } - } - } - - WriteChunk(output, type, data); - } - + byte[] data = TestFile.Create(path).Bytes; DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; - if (corruptEmptyChunk && integrity != SegmentIntegrityHandling.IgnoreImageData) + if (hasInvalidCrc && integrity != SegmentIntegrityHandling.IgnoreImageData) { InvalidImageContentException exception = Assert.Throws(() => { - using Image actual = Image.Load(options, output.ToArray()); + using Image actual = Image.Load(options, data); }); Assert.Equal("CRC Error. PNG IDAT chunk is corrupt!", exception.Message); return; } - using Image expected = Image.Load(encoded); - using Image actual = Image.Load(options, output.ToArray()); - + using Image expected = TestFile.Create(TestImages.Png.Noise).CreateRgba32Image(); + using Image actual = Image.Load(options, data); ImageComparer.Exact.VerifySimilarity(expected, actual); } /// - /// A frame data chunk may carry nothing but its sequence number. Such chunks must not end the frame's - /// compressed data early, wherever they fall. + /// Checks stored empty frame payloads without changing their pixels or bypassing image-data CRC policy. /// - /// The frame data layout to write. + /// The stored APNG input. /// The segment integrity policy. - /// Whether to corrupt the CRC of each sequence-only frame data chunk. + /// Whether the input contains invalid frame-data CRCs. [Theory] - [InlineData("LeadingEmpty", SegmentIntegrityHandling.Strict, false)] - [InlineData("ConsecutiveLeadingEmpty", SegmentIntegrityHandling.Strict, false)] - [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.Strict, false)] - [InlineData("TrailingEmpty", SegmentIntegrityHandling.Strict, false)] - [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.Strict, true)] - [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.IgnoreAncillary, true)] - [InlineData("EmptyWithinHeader", SegmentIntegrityHandling.IgnoreImageData, true)] - [InlineData("TruncatedSequenceBetween", SegmentIntegrityHandling.Strict, false)] - [InlineData("TruncatedSequenceBetween", SegmentIntegrityHandling.IgnoreAncillary, false)] - [InlineData("TruncatedSequenceBetween", SegmentIntegrityHandling.IgnoreImageData, false)] - public void Decode_EmptyFrameDataChunks_FollowsIntegrityPolicy(string layout, SegmentIntegrityHandling integrity, bool corruptEmptyChunk) + [InlineData(TestImages.Png.FdatLeadingEmpty, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.FdatConsecutiveLeadingEmpty, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.FdatEmptyWithinHeader, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.FdatTrailingEmpty, SegmentIntegrityHandling.Strict, false)] + [InlineData(TestImages.Png.FdatEmptyBadCrc, SegmentIntegrityHandling.Strict, true)] + [InlineData(TestImages.Png.FdatEmptyBadCrc, SegmentIntegrityHandling.IgnoreAncillary, true)] + [InlineData(TestImages.Png.FdatEmptyBadCrc, SegmentIntegrityHandling.IgnoreImageData, true)] + public void Decode_EmptyFrameDataChunks_FollowsIntegrityPolicy(string path, SegmentIntegrityHandling integrity, bool hasInvalidCrc) { - byte[] encoded = TestFile.Create(TestImages.Png.APng).Bytes; - List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); - Assert.Contains(chunks, c => c.Type == "fdAT"); - - // Renumber fcTL and fdAT together so rechunking preserves the animation's sequence order. - using MemoryStream output = new(); - output.Write(encoded.AsSpan(0, 8)); - uint sequenceNumber = 0; - foreach ((string type, byte[] data) in chunks) - { - if (type == "fdAT") - { - foreach (byte[] segment in SplitCompressedData(data[4..], layout)) - { - byte[] sequence = GetSequenceNumber(sequenceNumber++); - if (layout == "TruncatedSequenceBetween" && segment.Length == 0) - { - // The preceding payload reaches the inflater before this structural error is encountered. - WriteChunk(output, type, sequence.AsSpan(0, 3)); - } - else - { - WriteChunk(output, type, [.. sequence, .. segment]); - } - - if (corruptEmptyChunk && segment.Length == 0) - { - // fdAT is image data even though its chunk type is ancillary in the PNG naming scheme. - output.GetBuffer()[(int)output.Position - 1] ^= 1; - } - } - - continue; - } - - if (type == "fcTL") - { - BinaryPrimitives.WriteUInt32BigEndian(data, sequenceNumber++); - } - - WriteChunk(output, type, data); - } - + byte[] data = TestFile.Create(path).Bytes; DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; - if (layout == "TruncatedSequenceBetween") - { - Assert.Throws(() => - { - using Image actual = Image.Load(options, output.ToArray()); - }); - - Assert.Throws(() => Image.Identify(options, output.ToArray())); - return; - } - - if (corruptEmptyChunk && integrity != SegmentIntegrityHandling.IgnoreImageData) + if (hasInvalidCrc && integrity != SegmentIntegrityHandling.IgnoreImageData) { InvalidImageContentException exception = Assert.Throws(() => { - using Image actual = Image.Load(options, output.ToArray()); + using Image actual = Image.Load(options, data); }); Assert.Equal("CRC Error. PNG fdAT chunk is corrupt!", exception.Message); return; } - using Image expected = Image.Load(encoded); - using Image actual = Image.Load(options, output.ToArray()); + using Image expected = TestFile.Create(TestImages.Png.APng).CreateRgba32Image(); + using Image actual = Image.Load(options, data); ImageComparer.Exact.VerifySimilarity(expected, actual); } /// - /// Rejects frame data that cannot contain a sequence number, regardless of the recoverable-error policy. + /// Rejects stored frame chunks without a complete sequence number, including chunks within compressed data. /// - /// The invalid frame data length. + /// The malformed APNG input. /// The segment integrity policy. [Theory] - [InlineData(0, SegmentIntegrityHandling.Strict)] - [InlineData(3, SegmentIntegrityHandling.Strict)] - [InlineData(0, SegmentIntegrityHandling.IgnoreAncillary)] - [InlineData(3, SegmentIntegrityHandling.IgnoreAncillary)] - [InlineData(0, SegmentIntegrityHandling.IgnoreImageData)] - [InlineData(3, SegmentIntegrityHandling.IgnoreImageData)] - public void DecodeAndIdentify_FrameDataWithoutSequenceNumber_ThrowsInvalidImageContentException(int length, SegmentIntegrityHandling integrity) + [InlineData(TestImages.Png.FdatShort0, SegmentIntegrityHandling.Strict)] + [InlineData(TestImages.Png.FdatShort3, SegmentIntegrityHandling.Strict)] + [InlineData(TestImages.Png.FdatTruncatedSequenceBetween, SegmentIntegrityHandling.Strict)] + [InlineData(TestImages.Png.FdatShort0, SegmentIntegrityHandling.IgnoreAncillary)] + [InlineData(TestImages.Png.FdatShort3, SegmentIntegrityHandling.IgnoreAncillary)] + [InlineData(TestImages.Png.FdatTruncatedSequenceBetween, SegmentIntegrityHandling.IgnoreAncillary)] + [InlineData(TestImages.Png.FdatShort0, SegmentIntegrityHandling.IgnoreImageData)] + [InlineData(TestImages.Png.FdatShort3, SegmentIntegrityHandling.IgnoreImageData)] + [InlineData(TestImages.Png.FdatTruncatedSequenceBetween, SegmentIntegrityHandling.IgnoreImageData)] + public void DecodeAndIdentify_FrameDataWithoutSequenceNumber_ThrowsInvalidImageContentException(string path, SegmentIntegrityHandling integrity) { - byte[] encoded = TestFile.Create(TestImages.Png.APng).Bytes; - List<(string Type, byte[] Data)> chunks = ReadChunks(encoded); - Assert.Contains(chunks, chunk => chunk.Type == "fdAT"); - - using MemoryStream stream = new(); - stream.Write(encoded.AsSpan(0, 8)); - foreach ((string type, byte[] data) in chunks) - { - // Preserve valid CRCs so the failure must come from the missing structural field. - WriteChunk(stream, type, type == "fdAT" ? data.AsSpan(0, length) : data); - } - - byte[] invalid = stream.ToArray(); + byte[] data = TestFile.Create(path).Bytes; DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; InvalidImageContentException exception = Assert.Throws(() => { - using Image actual = Image.Load(options, invalid); + using Image actual = Image.Load(options, data); }); Assert.Equal("The frame data chunk does not contain a sequence number.", exception.Message); - Assert.Throws(() => Image.Identify(options, invalid)); + Assert.Throws(() => Image.Identify(options, data)); } /// - /// A palette chunk too short to hold one entry leaves no colors to index, so it is reported as a missing palette - /// instead of reading outside the color table. + /// Rejects stored indexed PNGs without one complete RGB palette entry in every integrity mode. /// - /// The interlace mode of the image. - /// The number of palette bytes, fewer than one complete entry. + /// The malformed PNG input, including its interlace mode and palette length. /// The segment integrity policy. [Theory] - [InlineData(PngInterlaceMode.None, 0, SegmentIntegrityHandling.Strict)] - [InlineData(PngInterlaceMode.None, 1, SegmentIntegrityHandling.Strict)] - [InlineData(PngInterlaceMode.None, 2, SegmentIntegrityHandling.Strict)] - [InlineData(PngInterlaceMode.Adam7, 0, SegmentIntegrityHandling.Strict)] - [InlineData(PngInterlaceMode.Adam7, 1, SegmentIntegrityHandling.Strict)] - [InlineData(PngInterlaceMode.Adam7, 2, SegmentIntegrityHandling.Strict)] - [InlineData(PngInterlaceMode.None, 2, SegmentIntegrityHandling.IgnoreAncillary)] - [InlineData(PngInterlaceMode.None, 2, SegmentIntegrityHandling.IgnoreImageData)] - public void Decode_PaletteChunkShorterThanOneEntry_ThrowsInvalidImageContentException( - PngInterlaceMode interlaceMode, - int paletteLength, - SegmentIntegrityHandling integrity) + [InlineData(TestImages.Png.PaletteShort0, SegmentIntegrityHandling.Strict)] + [InlineData(TestImages.Png.PaletteShort1, SegmentIntegrityHandling.Strict)] + [InlineData(TestImages.Png.PaletteShort2, SegmentIntegrityHandling.Strict)] + [InlineData(TestImages.Png.PaletteAdam7Short0, SegmentIntegrityHandling.Strict)] + [InlineData(TestImages.Png.PaletteAdam7Short1, SegmentIntegrityHandling.Strict)] + [InlineData(TestImages.Png.PaletteAdam7Short2, SegmentIntegrityHandling.Strict)] + [InlineData(TestImages.Png.PaletteShort2, SegmentIntegrityHandling.IgnoreAncillary)] + [InlineData(TestImages.Png.PaletteShort2, SegmentIntegrityHandling.IgnoreImageData)] + public void Decode_PaletteChunkShorterThanOneEntry_ThrowsInvalidImageContentException(string path, SegmentIntegrityHandling integrity) { - // Zeroed scanlines: filter type None and palette index 0. A 9x9 image has 9 rows of 1 + 9 bytes, or - // 100 bytes across the seven Adam7 passes, each row of which also starts with a filter byte. - byte[] indices = interlaceMode == PngInterlaceMode.Adam7 ? new byte[100] : new byte[90]; - using MemoryStream stream = new(); - stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); - WriteChunk(stream, "IHDR", CreateHeaderData(9, 9, 8, PngColorType.Palette, interlaceMode)); - WriteChunk(stream, "PLTE", new byte[paletteLength]); - WriteChunk(stream, "IDAT", Compress(indices)); - WriteChunk(stream, "IEND", []); - DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; Assert.Throws(() => { - using Image actual = Image.Load(options, stream.ToArray()); + using Image actual = Image.Load(options, TestFile.Create(path).Bytes); }); } /// - /// 16-bit grayscale tRNS samples are big-endian, so 0x0102 is transparent and its byte-swapped value is not. + /// Checks that the stored 16-bit marker 0x0102 selects that gray sample, not the byte-swapped sample 0x0201. /// [Fact] public void Decode_16BitGrayscaleTransparency_IsBigEndian() { - using MemoryStream stream = new(); - stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); - WriteChunk(stream, "IHDR", CreateHeaderData(2, 1, 16, PngColorType.Grayscale, PngInterlaceMode.None)); - WriteChunk(stream, "tRNS", [0x01, 0x02]); - WriteChunk(stream, "IDAT", Compress([0, 0x01, 0x02, 0x02, 0x01])); - WriteChunk(stream, "IEND", []); - byte[] data = stream.ToArray(); - + byte[] data = TestFile.Create(TestImages.Png.TrnsGray16BigEndian).Bytes; Assert.Equal(new L16(0x0102), Image.Identify(data).Metadata.GetPngMetadata().TransparentColor?.ToPixel()); using Image image = Image.Load(data); @@ -552,20 +430,12 @@ public void Decode_16BitGrayscaleTransparency_IsBigEndian() } /// - /// 16-bit truecolor tRNS samples are big-endian, so (0x0102, 0x0304, 0x0506) is transparent and its byte-swapped - /// value is not. + /// Checks that the stored marker (0x0102, 0x0304, 0x0506) selects that RGB sample, not its byte-swapped sample. /// [Fact] public void Decode_16BitRgbTransparency_IsBigEndian() { - using MemoryStream stream = new(); - stream.Write([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]); - WriteChunk(stream, "IHDR", CreateHeaderData(2, 1, 16, PngColorType.Rgb, PngInterlaceMode.None)); - WriteChunk(stream, "tRNS", [0x01, 0x02, 0x03, 0x04, 0x05, 0x06]); - WriteChunk(stream, "IDAT", Compress([0, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x02, 0x01, 0x04, 0x03, 0x06, 0x05])); - WriteChunk(stream, "IEND", []); - byte[] data = stream.ToArray(); - + byte[] data = TestFile.Create(TestImages.Png.TrnsRgb16BigEndian).Bytes; Rgb48 expected = new(0x0102, 0x0304, 0x0506); Assert.Equal(expected, Image.Identify(data).Metadata.GetPngMetadata().TransparentColor?.ToPixel()); @@ -574,117 +444,6 @@ public void Decode_16BitRgbTransparency_IsBigEndian() Assert.Equal(ushort.MaxValue, image[1, 0].A); } - /// - /// Splits compressed data without changing its bytes, including layouts with empty chunks. - /// - /// The complete compressed data. - /// The chunk layout. - /// The ordered chunk payloads. - private static byte[][] SplitCompressedData(byte[] data, string layout) - => layout switch - { - "LeadingEmpty" => [[], data], - "ConsecutiveLeadingEmpty" => [[], [], data], - "FirstOneByte" => [data[..1], data[1..]], - "EmptyWithinHeader" => [data[..1], [], data[1..]], - "EveryOneByte" => [.. data.Select(b => new[] { b })], - "EveryOneByteWithEmpty" => [.. data.SelectMany(b => new[] { [b], Array.Empty() })], - "EmptyBetween" or "TruncatedSequenceBetween" => [data[..(data.Length / 2)], [], data[(data.Length / 2)..]], - "TrailingEmpty" => [data, []], - _ => throw new ArgumentOutOfRangeException(nameof(layout)), - }; - - /// - /// Splits a PNG file into its chunks without validating them. - /// - /// The PNG file, including the signature. - /// The type and data of each chunk, in file order. - private static List<(string Type, byte[] Data)> ReadChunks(byte[] data) - { - List<(string Type, byte[] Data)> chunks = []; - int offset = 8; - while (offset + 12 <= data.Length) - { - int length = BinaryPrimitives.ReadInt32BigEndian(data.AsSpan(offset)); - chunks.Add((Encoding.ASCII.GetString(data, offset + 4, 4), data.AsSpan(offset + 8, length).ToArray())); - offset += length + 12; - } - - return chunks; - } - - /// - /// Writes a chunk with its length, type, data and a valid CRC. - /// - /// The stream to write to. - /// The four character chunk type. - /// The chunk data. - private static void WriteChunk(Stream stream, string type, ReadOnlySpan data) - { - Span buffer = stackalloc byte[4]; - BinaryPrimitives.WriteInt32BigEndian(buffer, data.Length); - stream.Write(buffer); - - // The CRC covers the chunk type and data but not the length. - byte[] typeBytes = Encoding.ASCII.GetBytes(type); - stream.Write(typeBytes); - stream.Write(data); - - Crc32 crc = new(); - crc.Append(typeBytes); - crc.Append(data); - BinaryPrimitives.WriteUInt32BigEndian(buffer, crc.GetCurrentHashAsUInt32()); - stream.Write(buffer); - } - - /// - /// Creates IHDR chunk data using compression method 0 and filter method 0. - /// - /// The image width. - /// The image height. - /// The bit depth. - /// The color type. - /// The interlace mode. - /// The chunk data. - private static byte[] CreateHeaderData(int width, int height, byte bitDepth, PngColorType colorType, PngInterlaceMode interlaceMode) - { - byte[] data = new byte[13]; - BinaryPrimitives.WriteInt32BigEndian(data, width); - BinaryPrimitives.WriteInt32BigEndian(data.AsSpan(4), height); - data[8] = bitDepth; - data[9] = (byte)colorType; - data[12] = (byte)interlaceMode; - return data; - } - - /// - /// Compresses filtered scanlines into a zlib stream. - /// - /// The filtered scanlines. - /// The zlib stream. - private static byte[] Compress(byte[] scanlines) - { - using MemoryStream output = new(); - using (ZLibStream zlib = new(output, CompressionLevel.Optimal, true)) - { - zlib.Write(scanlines); - } - - return output.ToArray(); - } - - /// - /// Gets the big-endian bytes of an APNG sequence number. - /// - /// The sequence number. - /// The encoded sequence number. - private static byte[] GetSequenceNumber(uint sequenceNumber) - { - byte[] data = new byte[4]; - BinaryPrimitives.WriteUInt32BigEndian(data, sequenceNumber); - return data; - } - private static string GetChunkTypeName(uint value) { byte[] data = new byte[4]; diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs index f1bb985e55..0a47eda2d5 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs @@ -358,14 +358,15 @@ public void ExcludeFilter_WithNone_DoesNotExcludeChunks() /// /// Checks the stored byte order independently of the decoder, then checks the identified transparency marker. /// + /// The stored 16-bit input image. /// The PNG color type. [Theory] - [InlineData(PngColorType.Grayscale)] - [InlineData(PngColorType.Rgb)] - public void TransparencyChunk_16Bit_IsWrittenBigEndian(PngColorType colorType) + [InlineData(TestImages.Png.TrnsGray16BigEndian, PngColorType.Grayscale)] + [InlineData(TestImages.Png.TrnsRgb16BigEndian, PngColorType.Rgb)] + public void TransparencyChunk_16Bit_IsWrittenBigEndian(string path, PngColorType colorType) { // arrange - using Image input = new(2, 1); + using Image input = Image.Load(TestFile.Create(path).Bytes); PngMetadata metadata = input.Metadata.GetPngMetadata(); metadata.ColorType = colorType; metadata.BitDepth = PngBitDepth.Bit16; diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index f070717d70..31dca8d5a6 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -66,6 +66,33 @@ public static class Png public const string AdamHeadsHlg = "Png/adamHeadsHLG.png"; public const string IptcMetadata = "Png/iptc-profile.png"; + // Chunk framing, integrity and transparency inputs. + public const string IdatLeadingEmpty = "Png/chunks/idat-leading-empty.png"; + public const string IdatConsecutiveLeadingEmpty = "Png/chunks/idat-consecutive-leading-empty.png"; + public const string IdatFirstOneByte = "Png/chunks/idat-first-one-byte.png"; + public const string IdatEmptyWithinHeader = "Png/chunks/idat-empty-within-header.png"; + public const string IdatEveryOneByte = "Png/chunks/idat-every-one-byte.png"; + public const string IdatEveryOneByteWithEmpty = "Png/chunks/idat-every-one-byte-with-empty.png"; + public const string IdatEmptyBetween = "Png/chunks/idat-empty-between.png"; + public const string IdatTrailingEmpty = "Png/chunks/idat-trailing-empty.png"; + public const string IdatEmptyBadCrc = "Png/chunks/idat-empty-bad-crc.png"; + public const string FdatLeadingEmpty = "Png/chunks/fdat-leading-empty.png"; + public const string FdatConsecutiveLeadingEmpty = "Png/chunks/fdat-consecutive-leading-empty.png"; + public const string FdatEmptyWithinHeader = "Png/chunks/fdat-empty-within-header.png"; + public const string FdatTrailingEmpty = "Png/chunks/fdat-trailing-empty.png"; + public const string FdatEmptyBadCrc = "Png/chunks/fdat-empty-bad-crc.png"; + public const string FdatTruncatedSequenceBetween = "Png/chunks/fdat-truncated-sequence-between.png"; + public const string FdatShort0 = "Png/chunks/fdat-short-0.png"; + public const string FdatShort3 = "Png/chunks/fdat-short-3.png"; + public const string PaletteShort0 = "Png/chunks/palette-short-0.png"; + public const string PaletteShort1 = "Png/chunks/palette-short-1.png"; + public const string PaletteShort2 = "Png/chunks/palette-short-2.png"; + public const string PaletteAdam7Short0 = "Png/chunks/palette-adam7-short-0.png"; + public const string PaletteAdam7Short1 = "Png/chunks/palette-adam7-short-1.png"; + public const string PaletteAdam7Short2 = "Png/chunks/palette-adam7-short-2.png"; + public const string TrnsGray16BigEndian = "Png/chunks/trns-gray16-big-endian.png"; + public const string TrnsRgb16BigEndian = "Png/chunks/trns-rgb16-big-endian.png"; + // Animated // https://philip.html5.org/tests/apng/tests.html public const string APng = "Png/animated/apng.png"; diff --git a/tests/Images/Input/Png/chunks/fdat-consecutive-leading-empty.png b/tests/Images/Input/Png/chunks/fdat-consecutive-leading-empty.png new file mode 100644 index 0000000000..8f394e2a1c --- /dev/null +++ b/tests/Images/Input/Png/chunks/fdat-consecutive-leading-empty.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:79f77899925f8f9b77309d6d392090346889a2bd7f7b3b38595c2af10fe26060 +size 2636 diff --git a/tests/Images/Input/Png/chunks/fdat-empty-bad-crc.png b/tests/Images/Input/Png/chunks/fdat-empty-bad-crc.png new file mode 100644 index 0000000000..9861b0232a --- /dev/null +++ b/tests/Images/Input/Png/chunks/fdat-empty-bad-crc.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:25419ca02b75f7be52e31601e9b5525fd1554bad430747c1a5a7eb2fdf92c5d0 +size 2636 diff --git a/tests/Images/Input/Png/chunks/fdat-empty-within-header.png b/tests/Images/Input/Png/chunks/fdat-empty-within-header.png new file mode 100644 index 0000000000..6e679b3f3c --- /dev/null +++ b/tests/Images/Input/Png/chunks/fdat-empty-within-header.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4a9f21fdcad7d91e82cbad349e25d3e40f6f9ab78e085e3960a5bdc410f6f0ca +size 2636 diff --git a/tests/Images/Input/Png/chunks/fdat-leading-empty.png b/tests/Images/Input/Png/chunks/fdat-leading-empty.png new file mode 100644 index 0000000000..7e2c1f19d2 --- /dev/null +++ b/tests/Images/Input/Png/chunks/fdat-leading-empty.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:98eeca49aa75598628c2edd9f24179f3e76a6f139495de0a389b704c002a047a +size 2572 diff --git a/tests/Images/Input/Png/chunks/fdat-short-0.png b/tests/Images/Input/Png/chunks/fdat-short-0.png new file mode 100644 index 0000000000..db433236aa --- /dev/null +++ b/tests/Images/Input/Png/chunks/fdat-short-0.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:719690c12c5770ea6aa959787ba21a5a8ccd551f4e75f1a543f593abac3c6aea +size 1417 diff --git a/tests/Images/Input/Png/chunks/fdat-short-3.png b/tests/Images/Input/Png/chunks/fdat-short-3.png new file mode 100644 index 0000000000..3ab27d75f9 --- /dev/null +++ b/tests/Images/Input/Png/chunks/fdat-short-3.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:5fcaa5d6925d3c35e9e9102d797b96ab2621bab0bc48747bd261a99bc46341a5 +size 1429 diff --git a/tests/Images/Input/Png/chunks/fdat-trailing-empty.png b/tests/Images/Input/Png/chunks/fdat-trailing-empty.png new file mode 100644 index 0000000000..8d82d3655d --- /dev/null +++ b/tests/Images/Input/Png/chunks/fdat-trailing-empty.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:68999c390cd9a26119fb7fe0e694d95e862ba3b5fb4d380b40229ef75b9c9128 +size 2572 diff --git a/tests/Images/Input/Png/chunks/fdat-truncated-sequence-between.png b/tests/Images/Input/Png/chunks/fdat-truncated-sequence-between.png new file mode 100644 index 0000000000..07462ed096 --- /dev/null +++ b/tests/Images/Input/Png/chunks/fdat-truncated-sequence-between.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:3de847e39d88d93482979c7a799d24d1ff71ee9b14828ccbdb629397ae683c90 +size 2632 diff --git a/tests/Images/Input/Png/chunks/idat-consecutive-leading-empty.png b/tests/Images/Input/Png/chunks/idat-consecutive-leading-empty.png new file mode 100644 index 0000000000..c152e6a1d4 --- /dev/null +++ b/tests/Images/Input/Png/chunks/idat-consecutive-leading-empty.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0cc1e6e5435e61588c395787343dd3794121b560fa88aa0ca9fc4c4a70517781 +size 4922 diff --git a/tests/Images/Input/Png/chunks/idat-empty-bad-crc.png b/tests/Images/Input/Png/chunks/idat-empty-bad-crc.png new file mode 100644 index 0000000000..076fdb91c6 --- /dev/null +++ b/tests/Images/Input/Png/chunks/idat-empty-bad-crc.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:aed0d412a8607f7d3407411c065ef16b6a8295b30e9ced3653160692b20df010 +size 4922 diff --git a/tests/Images/Input/Png/chunks/idat-empty-between.png b/tests/Images/Input/Png/chunks/idat-empty-between.png new file mode 100644 index 0000000000..a2a7001da7 --- /dev/null +++ b/tests/Images/Input/Png/chunks/idat-empty-between.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b6f95d596f54a3ffb33d59e6f95371b895b722dc0ae3e9d54c4d472e5652e3b2 +size 4922 diff --git a/tests/Images/Input/Png/chunks/idat-empty-within-header.png b/tests/Images/Input/Png/chunks/idat-empty-within-header.png new file mode 100644 index 0000000000..d2d16ac585 --- /dev/null +++ b/tests/Images/Input/Png/chunks/idat-empty-within-header.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:bd86ee484684f1d7a7d705914bfb6feb91e5c89cfa221b861315e440ebd27a2e +size 4922 diff --git a/tests/Images/Input/Png/chunks/idat-every-one-byte-with-empty.png b/tests/Images/Input/Png/chunks/idat-every-one-byte-with-empty.png new file mode 100644 index 0000000000..0188d1df9a --- /dev/null +++ b/tests/Images/Input/Png/chunks/idat-every-one-byte-with-empty.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:56fbdd00aebeadc5b85509b89c9ee3a160b5f5f4c17d283c7298e606b55581e8 +size 121070 diff --git a/tests/Images/Input/Png/chunks/idat-every-one-byte.png b/tests/Images/Input/Png/chunks/idat-every-one-byte.png new file mode 100644 index 0000000000..dbcd1f7061 --- /dev/null +++ b/tests/Images/Input/Png/chunks/idat-every-one-byte.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:937fcd40242fe8faefb8106d9d6ec3134e812778161b3946596a085cd0a1cf15 +size 62978 diff --git a/tests/Images/Input/Png/chunks/idat-first-one-byte.png b/tests/Images/Input/Png/chunks/idat-first-one-byte.png new file mode 100644 index 0000000000..355bb94d67 --- /dev/null +++ b/tests/Images/Input/Png/chunks/idat-first-one-byte.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:691283f42518be72a1e60a4555f62827a044e63423db7a8f1445238b00a5a24c +size 4910 diff --git a/tests/Images/Input/Png/chunks/idat-leading-empty.png b/tests/Images/Input/Png/chunks/idat-leading-empty.png new file mode 100644 index 0000000000..45fafa00d8 --- /dev/null +++ b/tests/Images/Input/Png/chunks/idat-leading-empty.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d695fd60e2405d44623484a5cbc78c10bbdd84a15dd705234dfd6b90a1b30cb4 +size 4910 diff --git a/tests/Images/Input/Png/chunks/idat-trailing-empty.png b/tests/Images/Input/Png/chunks/idat-trailing-empty.png new file mode 100644 index 0000000000..afa97ef140 --- /dev/null +++ b/tests/Images/Input/Png/chunks/idat-trailing-empty.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:abd1a897f8ea4d3253d137f14705643e6ad5572b2aeca85e41536831ac97b997 +size 4910 diff --git a/tests/Images/Input/Png/chunks/palette-adam7-short-0.png b/tests/Images/Input/Png/chunks/palette-adam7-short-0.png new file mode 100644 index 0000000000..74c5734294 --- /dev/null +++ b/tests/Images/Input/Png/chunks/palette-adam7-short-0.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:acd3c2dc78112c2801008944cd5504db2d94b958cf3dd95065d982f7842957b5 +size 81 diff --git a/tests/Images/Input/Png/chunks/palette-adam7-short-1.png b/tests/Images/Input/Png/chunks/palette-adam7-short-1.png new file mode 100644 index 0000000000..6056a742dd --- /dev/null +++ b/tests/Images/Input/Png/chunks/palette-adam7-short-1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:408e6d68c4385b07ed184f066180e0491f08bf06999476f81fe9b5bcd3833281 +size 82 diff --git a/tests/Images/Input/Png/chunks/palette-adam7-short-2.png b/tests/Images/Input/Png/chunks/palette-adam7-short-2.png new file mode 100644 index 0000000000..428bfaa23c --- /dev/null +++ b/tests/Images/Input/Png/chunks/palette-adam7-short-2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1d91e38273e814dbf5df7633a8c2db30264746b2179aff04c8946c3eeeb7ab31 +size 83 diff --git a/tests/Images/Input/Png/chunks/palette-short-0.png b/tests/Images/Input/Png/chunks/palette-short-0.png new file mode 100644 index 0000000000..9aa8726fc5 --- /dev/null +++ b/tests/Images/Input/Png/chunks/palette-short-0.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ca328183eca2bebd5fcff5ca0a17c0781e6465780b022ec2baa41bcc38465108 +size 81 diff --git a/tests/Images/Input/Png/chunks/palette-short-1.png b/tests/Images/Input/Png/chunks/palette-short-1.png new file mode 100644 index 0000000000..7d29e3864c --- /dev/null +++ b/tests/Images/Input/Png/chunks/palette-short-1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4d6ff9e835c14793099cc665b121e52f9e246633380a4a5f506f43e42bff1371 +size 82 diff --git a/tests/Images/Input/Png/chunks/palette-short-2.png b/tests/Images/Input/Png/chunks/palette-short-2.png new file mode 100644 index 0000000000..321b347ec1 --- /dev/null +++ b/tests/Images/Input/Png/chunks/palette-short-2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1d9a6bb7f0774bde7633b53042fd3b6c43a5d9ec61d9d2c6a2f6ba49afad4fd3 +size 83 diff --git a/tests/Images/Input/Png/chunks/trns-gray16-big-endian.png b/tests/Images/Input/Png/chunks/trns-gray16-big-endian.png new file mode 100644 index 0000000000..4bd5bd6fb5 --- /dev/null +++ b/tests/Images/Input/Png/chunks/trns-gray16-big-endian.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:30035517659d0c0a1a27af315927c073fef6173a20f4fddd3e7cce105a7a30fa +size 84 diff --git a/tests/Images/Input/Png/chunks/trns-rgb16-big-endian.png b/tests/Images/Input/Png/chunks/trns-rgb16-big-endian.png new file mode 100644 index 0000000000..c95cb8c244 --- /dev/null +++ b/tests/Images/Input/Png/chunks/trns-rgb16-big-endian.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cc1e7a3bc53545da4c41967daaf694b55a6c32dffa080a09b57b9ea67f108ce7 +size 96