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); + } }