diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index 399bc95c92..89352ff7e9 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -10,7 +10,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// internal readonly struct PngChunk { - public PngChunk(int length, string type, IManagedByteBuffer data = null, uint crc = default) + public PngChunk(int length, PngChunkType type, IManagedByteBuffer data = null, uint crc = default) { this.Length = length; this.Type = type; @@ -27,9 +27,10 @@ namespace SixLabors.ImageSharp.Formats.Png public int Length { get; } /// - /// Gets the chunk type as string with 4 chars. + /// Gets the chunk type. + /// The chunk type value the UInt32BigEndian encoding of its 4 ASCII characters. /// - public string Type { get; } + public PngChunkType Type { get; } /// /// Gets the data bytes appropriate to the chunk type, if any. @@ -48,9 +49,9 @@ namespace SixLabors.ImageSharp.Formats.Png /// Gets a value indicating whether the given chunk is critical to decoding /// public bool IsCritical => - this.Type == PngChunkTypes.Header || - this.Type == PngChunkTypes.Palette || - this.Type == PngChunkTypes.Data || - this.Type == PngChunkTypes.End; + this.Type == PngChunkType.Header || + this.Type == PngChunkType.Palette || + this.Type == PngChunkType.Data || + this.Type == PngChunkType.End; } } diff --git a/src/ImageSharp/Formats/Png/PngChunkType.cs b/src/ImageSharp/Formats/Png/PngChunkType.cs new file mode 100644 index 0000000000..14550e8aca --- /dev/null +++ b/src/ImageSharp/Formats/Png/PngChunkType.cs @@ -0,0 +1,17 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +namespace SixLabors.ImageSharp.Formats.Png +{ + internal enum PngChunkType : uint + { + Header = 1229472850U, // IHDR + Palette = 1347179589U, // PLTE + Data = 1229209940U, // IDAT + End = 1229278788U, // IEND + PaletteAlpha = 1951551059U, // tRNS + Text = 1950701684U, // tEXt + Gamma = 1732332865U, // gAMA + Physical = 1883789683U, // pHYs + } +} diff --git a/src/ImageSharp/Formats/Png/PngChunkTypes.cs b/src/ImageSharp/Formats/Png/PngChunkTypeNames.cs similarity index 95% rename from src/ImageSharp/Formats/Png/PngChunkTypes.cs rename to src/ImageSharp/Formats/Png/PngChunkTypeNames.cs index e22f4f0e7d..f2864decd6 100644 --- a/src/ImageSharp/Formats/Png/PngChunkTypes.cs +++ b/src/ImageSharp/Formats/Png/PngChunkTypeNames.cs @@ -4,9 +4,9 @@ namespace SixLabors.ImageSharp.Formats.Png { /// - /// Contains a list of possible chunk type identifiers. + /// Contains a list of possible chunk type identifier names. /// - internal static class PngChunkTypes + internal static class PngChunkTypeNames { /// /// The first chunk in a png file. Can only exists once. Contains diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index b27d9a9659..053e9f712b 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -71,11 +71,6 @@ namespace SixLabors.ImageSharp.Formats.Png /// private readonly byte[] crcBuffer = new byte[4]; - /// - /// Reusable buffer for reading char arrays. - /// - private readonly char[] chars = new char[4]; - /// /// Reusable crc for validating chunks. /// @@ -224,14 +219,14 @@ namespace SixLabors.ImageSharp.Formats.Png { switch (chunk.Type) { - case PngChunkTypes.Header: + case PngChunkType.Header: this.ReadHeaderChunk(chunk.Data.Array); this.ValidateHeader(); break; - case PngChunkTypes.Physical: + case PngChunkType.Physical: this.ReadPhysicalChunk(metadata, chunk.Data.Array); break; - case PngChunkTypes.Data: + case PngChunkType.Data: if (image == null) { this.InitializeImage(metadata, out image); @@ -241,21 +236,21 @@ namespace SixLabors.ImageSharp.Formats.Png this.ReadScanlines(deframeStream.CompressedStream, image.Frames.RootFrame); this.currentStream.Read(this.crcBuffer, 0, 4); break; - case PngChunkTypes.Palette: + case PngChunkType.Palette: byte[] pal = new byte[chunk.Length]; Buffer.BlockCopy(chunk.Data.Array, 0, pal, 0, chunk.Length); this.palette = pal; break; - case PngChunkTypes.PaletteAlpha: + case PngChunkType.PaletteAlpha: byte[] alpha = new byte[chunk.Length]; Buffer.BlockCopy(chunk.Data.Array, 0, alpha, 0, chunk.Length); this.paletteAlpha = alpha; this.AssignTransparentMarkers(alpha); break; - case PngChunkTypes.Text: + case PngChunkType.Text: this.ReadTextChunk(metadata, chunk.Data.Array, chunk.Length); break; - case PngChunkTypes.End: + case PngChunkType.End: this.isEndChunkReached = true; break; } @@ -298,20 +293,20 @@ namespace SixLabors.ImageSharp.Formats.Png { switch (chunk.Type) { - case PngChunkTypes.Header: + case PngChunkType.Header: this.ReadHeaderChunk(chunk.Data.Array); this.ValidateHeader(); break; - case PngChunkTypes.Physical: + case PngChunkType.Physical: this.ReadPhysicalChunk(metadata, chunk.Data.Array); break; - case PngChunkTypes.Data: + case PngChunkType.Data: this.SkipChunkDataAndCrc(chunk); break; - case PngChunkTypes.Text: + case PngChunkType.Text: this.ReadTextChunk(metadata, chunk.Data.Array, chunk.Length); break; - case PngChunkTypes.End: + case PngChunkType.End: this.isEndChunkReached = true; break; } @@ -1214,10 +1209,10 @@ namespace SixLabors.ImageSharp.Formats.Png } } - string type = this.ReadChunkType(); + PngChunkType type = this.ReadChunkType(); // NOTE: Reading the chunk data is the responsible of the caller - if (type == PngChunkTypes.Data) + if (type == PngChunkType.Data) { chunk = new PngChunk(length, type); @@ -1246,7 +1241,9 @@ namespace SixLabors.ImageSharp.Formats.Png if (this.crc.Value != chunk.Crc) { - throw new ImageFormatException($"CRC Error. PNG {chunk.Type} chunk is corrupt!"); + string chunkName = Encoding.UTF8.GetString(this.chunkTypeBuffer, 0, 4); + + throw new ImageFormatException($"CRC Error. PNG {chunkName} chunk is corrupt!"); } } @@ -1297,20 +1294,16 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Thrown if the input stream is not valid. /// - private string ReadChunkType() + private PngChunkType ReadChunkType() { int numBytes = this.currentStream.Read(this.chunkTypeBuffer, 0, 4); + if (numBytes >= 1 && numBytes <= 3) { throw new ImageFormatException("Image stream is not valid!"); } - this.chars[0] = (char)this.chunkTypeBuffer[0]; - this.chars[1] = (char)this.chunkTypeBuffer[1]; - this.chars[2] = (char)this.chunkTypeBuffer[2]; - this.chars[3] = (char)this.chunkTypeBuffer[3]; - - return new string(this.chars); + return (PngChunkType)BinaryPrimitives.ReadUInt32BigEndian(this.chunkTypeBuffer.AsSpan()); } /// diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index e8a42c0c87..96c6a66507 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -27,9 +27,9 @@ namespace SixLabors.ImageSharp.Formats.Png private const int MaxBlockSize = 65535; /// - /// Reusable buffer for writing chunk types. + /// Reusable buffer for writing general data. /// - private readonly byte[] chunkTypeBuffer = new byte[4]; + private readonly byte[] buffer = new byte[8]; /// /// Reusable buffer for writing chunk data. @@ -423,7 +423,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.chunkDataBuffer[11] = header.FilterMethod; this.chunkDataBuffer[12] = (byte)header.InterlaceMethod; - this.WriteChunk(stream, PngChunkTypes.Header, this.chunkDataBuffer, 0, 13); + this.WriteChunk(stream, PngChunkType.Header, this.chunkDataBuffer, 0, 13); } /// @@ -474,12 +474,12 @@ namespace SixLabors.ImageSharp.Formats.Png } } - this.WriteChunk(stream, PngChunkTypes.Palette, colorTable.Array, 0, colorTableLength); + this.WriteChunk(stream, PngChunkType.Palette, colorTable.Array, 0, colorTableLength); // Write the transparency data if (anyAlpha) { - this.WriteChunk(stream, PngChunkTypes.PaletteAlpha, alphaTable.Array, 0, pixelCount); + this.WriteChunk(stream, PngChunkType.PaletteAlpha, alphaTable.Array, 0, pixelCount); } } } @@ -504,7 +504,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.chunkDataBuffer[8] = 1; - this.WriteChunk(stream, PngChunkTypes.Physical, this.chunkDataBuffer, 0, 9); + this.WriteChunk(stream, PngChunkType.Physical, this.chunkDataBuffer, 0, 9); } } @@ -521,7 +521,7 @@ namespace SixLabors.ImageSharp.Formats.Png BinaryPrimitives.WriteUInt32BigEndian(this.chunkDataBuffer.AsSpan(0, 4), gammaValue); - this.WriteChunk(stream, PngChunkTypes.Gamma, this.chunkDataBuffer, 0, 4); + this.WriteChunk(stream, PngChunkType.Gamma, this.chunkDataBuffer, 0, 4); } } @@ -589,7 +589,7 @@ namespace SixLabors.ImageSharp.Formats.Png length = MaxBlockSize; } - this.WriteChunk(stream, PngChunkTypes.Data, buffer, i * MaxBlockSize, length); + this.WriteChunk(stream, PngChunkType.Data, buffer, i * MaxBlockSize, length); } } @@ -599,7 +599,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// The containing image data. private void WriteEndChunk(Stream stream) { - this.WriteChunk(stream, PngChunkTypes.End, null); + this.WriteChunk(stream, PngChunkType.End, null); } /// @@ -608,7 +608,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// The to write to. /// The type of chunk to write. /// The containing data. - private void WriteChunk(Stream stream, string type, byte[] data) + private void WriteChunk(Stream stream, PngChunkType type, byte[] data) { this.WriteChunk(stream, type, data, 0, data?.Length ?? 0); } @@ -621,22 +621,16 @@ namespace SixLabors.ImageSharp.Formats.Png /// The containing data. /// The position to offset the data at. /// The of the data to write. - private void WriteChunk(Stream stream, string type, byte[] data, int offset, int length) + private void WriteChunk(Stream stream, PngChunkType type, byte[] data, int offset, int length) { - BinaryPrimitives.WriteInt32BigEndian(this.intBuffer, length); + BinaryPrimitives.WriteInt32BigEndian(this.buffer, length); + BinaryPrimitives.WriteUInt32BigEndian(this.buffer.AsSpan(4, 4), (uint)type); - stream.Write(this.intBuffer, 0, 4); // write the length - - this.chunkTypeBuffer[0] = (byte)type[0]; - this.chunkTypeBuffer[1] = (byte)type[1]; - this.chunkTypeBuffer[2] = (byte)type[2]; - this.chunkTypeBuffer[3] = (byte)type[3]; - - stream.Write(this.chunkTypeBuffer, 0, 4); + stream.Write(this.buffer, 0, 8); this.crc.Reset(); - this.crc.Update(this.chunkTypeBuffer); + this.crc.Update(this.buffer.AsSpan(4, 4)); // Write the type buffer if (data != null && length > 0) { diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 1de4e16467..85430fea9c 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -242,10 +242,10 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [InlineData(PngChunkTypes.Header)] - [InlineData(PngChunkTypes.Palette)] + [InlineData(PngChunkTypeNames.Header)] + [InlineData(PngChunkTypeNames.Palette)] // [InlineData(PngChunkTypes.Data)] //TODO: Figure out how to test this - [InlineData(PngChunkTypes.End)] + [InlineData(PngChunkTypeNames.End)] public void Decode_IncorrectCRCForCriticalChunk_ExceptionIsThrown(string chunkName) { using (var memStream = new MemoryStream()) @@ -266,9 +266,9 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [InlineData(PngChunkTypes.Gamma)] - [InlineData(PngChunkTypes.PaletteAlpha)] - [InlineData(PngChunkTypes.Physical)] // It's ok to test physical as we don't throw for duplicate chunks. + [InlineData(PngChunkTypeNames.Gamma)] + [InlineData(PngChunkTypeNames.PaletteAlpha)] + [InlineData(PngChunkTypeNames.Physical)] // It's ok to test physical as we don't throw for duplicate chunks. //[InlineData(PngChunkTypes.Text)] //TODO: Figure out how to test this public void Decode_IncorrectCRCForNonCriticalChunk_ExceptionIsThrown(string chunkName) {