Merge pull request #1188 from SixLabors/bp/chunkOrder

Gamma chunk should come before palette chunk

Author: JimBobSquarePants

src/ImageSharp/Formats/Png/PngEncoderCore.cs

@@ -149,10 +149,10 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
             stream.Write(PngConstants.HeaderBytes);
 
             this.WriteHeaderChunk(stream);
+            this.WriteGammaChunk(stream);
             this.WritePaletteChunk(stream, quantized);
             this.WriteTransparencyChunk(stream, pngMetadata);
             this.WritePhysicalChunk(stream, metadata);
-            this.WriteGammaChunk(stream);
             this.WriteExifChunk(stream, metadata);
             this.WriteTextChunks(stream, pngMetadata);
             this.WriteDataChunks(image.Frames.RootFrame, quantized, stream);
@@ -538,6 +538,7 @@ private void WriteHeaderChunk(Stream stream)
 
         /// <summary>
         /// Writes the palette chunk to the stream.
+        /// Should be written before the first IDAT chunk.
         /// </summary>
         /// <typeparam name="TPixel">The pixel format.</typeparam>
         /// <param name="stream">The <see cref="Stream"/> containing image data.</param>
@@ -595,6 +596,7 @@ private void WritePaletteChunk<TPixel>(Stream stream, IndexedImageFrame<TPixel>
 
         /// <summary>
         /// Writes the physical dimension information to the stream.
+        /// Should be written before IDAT chunk.
         /// </summary>
         /// <param name="stream">The <see cref="Stream"/> containing image data.</param>
         /// <param name="meta">The image metadata.</param>
@@ -716,6 +718,7 @@ private byte[] GetCompressedTextBytes(byte[] textBytes)
 
         /// <summary>
         /// Writes the gamma information to the stream.
+        /// Should be written before PLTE and IDAT chunk.
         /// </summary>
         /// <param name="stream">The <see cref="Stream"/> containing image data.</param>
         private void WriteGammaChunk(Stream stream)
@@ -733,6 +736,7 @@ private void WriteGammaChunk(Stream stream)
 
         /// <summary>
         /// Writes the transparency chunk to the stream.
+        /// Should be written after PLTE and before IDAT.
         /// </summary>
         /// <param name="stream">The <see cref="Stream"/> containing image data.</param>
         /// <param name="pngMetadata">The image metadata.</param>

tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs

@@ -2,6 +2,8 @@
 // Licensed under the Apache License, Version 2.0.
 
 // ReSharper disable InconsistentNaming
+using System;
+using System.Buffers.Binary;
 using System.IO;
 using System.Linq;
 
@@ -18,6 +20,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png
 {
     public class PngEncoderTests
     {
+        private static PngEncoder PngEncoder => new PngEncoder();
+
         public static readonly TheoryData<string, PngBitDepth> PngBitDepthFiles =
         new TheoryData<string, PngBitDepth>
         {
@@ -234,8 +238,7 @@ public void InfersColorTypeAndBitDepth<TPixel>(TestImageProvider<TPixel> provide
         {
             using (Stream stream = new MemoryStream())
             {
-                var encoder = new PngEncoder();
-                encoder.Encode(provider.GetImage(), stream);
+                PngEncoder.Encode(provider.GetImage(), stream);
 
                 stream.Seek(0, SeekOrigin.Begin);
 
@@ -281,7 +284,7 @@ public void WritesFileMarker<TPixel>(TestImageProvider<TPixel> provider)
             using (Image<TPixel> image = provider.GetImage())
             using (var ms = new MemoryStream())
             {
-                image.Save(ms, new PngEncoder());
+                image.Save(ms, PngEncoder);
 
                 byte[] data = ms.ToArray().Take(8).ToArray();
                 byte[] expected =
@@ -304,14 +307,12 @@ public void WritesFileMarker<TPixel>(TestImageProvider<TPixel> provider)
         [MemberData(nameof(RatioFiles))]
         public void Encode_PreserveRatio(string imagePath, int xResolution, int yResolution, PixelResolutionUnit resolutionUnit)
         {
-            var options = new PngEncoder();
-
             var testFile = TestFile.Create(imagePath);
             using (Image<Rgba32> input = testFile.CreateRgba32Image())
             {
                 using (var memStream = new MemoryStream())
                 {
-                    input.Save(memStream, options);
+                    input.Save(memStream, PngEncoder);
 
                     memStream.Position = 0;
                     using (var output = Image.Load<Rgba32>(memStream))
@@ -329,14 +330,12 @@ public void Encode_PreserveRatio(string imagePath, int xResolution, int yResolut
         [MemberData(nameof(PngBitDepthFiles))]
         public void Encode_PreserveBits(string imagePath, PngBitDepth pngBitDepth)
         {
-            var options = new PngEncoder();
-
             var testFile = TestFile.Create(imagePath);
             using (Image<Rgba32> input = testFile.CreateRgba32Image())
             {
                 using (var memStream = new MemoryStream())
                 {
-                    input.Save(memStream, options);
+                    input.Save(memStream, PngEncoder);
 
                     memStream.Position = 0;
                     using (var output = Image.Load<Rgba32>(memStream))
@@ -353,8 +352,6 @@ public void Encode_PreserveBits(string imagePath, PngBitDepth pngBitDepth)
         [MemberData(nameof(PngTrnsFiles))]
         public void Encode_PreserveTrns(string imagePath, PngBitDepth pngBitDepth, PngColorType pngColorType)
         {
-            var options = new PngEncoder();
-
             var testFile = TestFile.Create(imagePath);
             using (Image<Rgba32> input = testFile.CreateRgba32Image())
             {
@@ -363,7 +360,7 @@ public void Encode_PreserveTrns(string imagePath, PngBitDepth pngBitDepth, PngCo
 
                 using (var memStream = new MemoryStream())
                 {
-                    input.Save(memStream, options);
+                    input.Save(memStream, PngEncoder);
                     memStream.Position = 0;
                     using (var output = Image.Load<Rgba32>(memStream))
                     {
@@ -404,6 +401,126 @@ public void Encode_PreserveTrns(string imagePath, PngBitDepth pngBitDepth, PngCo
             }
         }
 
+        [Fact]
+        public void HeaderChunk_ComesFirst()
+        {
+            var testFile = TestFile.Create(TestImages.Png.PngWithMetadata);
+            using Image<Rgba32> input = testFile.CreateRgba32Image();
+            using var memStream = new MemoryStream();
+            input.Save(memStream, PngEncoder);
+            memStream.Position = 0;
+
+            // Skip header.
+            Span<byte> bytesSpan = memStream.ToArray().AsSpan(8);
+            BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(0, 4));
+            var type = (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(4, 4));
+            Assert.Equal(PngChunkType.Header, type);
+        }
+
+        [Fact]
+        public void EndChunk_IsLast()
+        {
+            var testFile = TestFile.Create(TestImages.Png.PngWithMetadata);
+            using Image<Rgba32> input = testFile.CreateRgba32Image();
+            using var memStream = new MemoryStream();
+            input.Save(memStream, PngEncoder);
+            memStream.Position = 0;
+
+            // Skip header.
+            Span<byte> bytesSpan = memStream.ToArray().AsSpan(8);
+
+            bool endChunkFound = false;
+            while (bytesSpan.Length > 0)
+            {
+                int length = BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(0, 4));
+                var type = (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(4, 4));
+                Assert.False(endChunkFound);
+                if (type == PngChunkType.End)
+                {
+                    endChunkFound = true;
+                }
+
+                bytesSpan = bytesSpan.Slice(4 + 4 + length + 4);
+            }
+        }
+
+        [Theory]
+        [InlineData(PngChunkType.Gamma)]
+        [InlineData(PngChunkType.Chroma)]
+        [InlineData(PngChunkType.EmbeddedColorProfile)]
+        [InlineData(PngChunkType.SignificantBits)]
+        [InlineData(PngChunkType.StandardRgbColourSpace)]
+        public void Chunk_ComesBeforePlteAndIDat(object chunkTypeObj)
+        {
+            var chunkType = (PngChunkType)chunkTypeObj;
+            var testFile = TestFile.Create(TestImages.Png.PngWithMetadata);
+            using Image<Rgba32> input = testFile.CreateRgba32Image();
+            using var memStream = new MemoryStream();
+            input.Save(memStream, PngEncoder);
+            memStream.Position = 0;
+
+            // Skip header.
+            Span<byte> bytesSpan = memStream.ToArray().AsSpan(8);
+
+            bool palFound = false;
+            bool dataFound = false;
+            while (bytesSpan.Length > 0)
+            {
+                int length = BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(0, 4));
+                var type = (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(4, 4));
+                if (chunkType == type)
+                {
+                    Assert.False(palFound || dataFound, $"{chunkType} chunk should come before data and palette chunk");
+                }
+
+                switch (type)
+                {
+                    case PngChunkType.Data:
+                        dataFound = true;
+                        break;
+                    case PngChunkType.Palette:
+                        palFound = true;
+                        break;
+                }
+
+                bytesSpan = bytesSpan.Slice(4 + 4 + length + 4);
+            }
+        }
+
+        [Theory]
+        [InlineData(PngChunkType.Physical)]
+        [InlineData(PngChunkType.SuggestedPalette)]
+        public void Chunk_ComesBeforeIDat(object chunkTypeObj)
+        {
+            var chunkType = (PngChunkType)chunkTypeObj;
+            var testFile = TestFile.Create(TestImages.Png.PngWithMetadata);
+            using Image<Rgba32> input = testFile.CreateRgba32Image();
+            using var memStream = new MemoryStream();
+            input.Save(memStream, PngEncoder);
+            memStream.Position = 0;
+
+            // Skip header.
+            Span<byte> bytesSpan = memStream.ToArray().AsSpan(8);
+
+            bool dataFound = false;
+            while (bytesSpan.Length > 0)
+            {
+                int length = BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(0, 4));
+                var type = (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(4, 4));
+                if (chunkType == type)
+                {
+                    Assert.False(dataFound, $"{chunkType} chunk should come before data chunk");
+                }
+
+                if (type == PngChunkType.Data)
+                {
+                    dataFound = true;
+                }
+
+                bytesSpan = bytesSpan.Slice(4 + 4 + length + 4);
+            }
+        }
+
         [Theory]
         [WithTestPatternImages(587, 821, PixelTypes.Rgba32)]
         [WithTestPatternImages(677, 683, PixelTypes.Rgba32)]