From 76568a7093ddc9874b20e15771d8e15e7a733c71 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Thu, 12 Apr 2018 10:33:19 -0700 Subject: [PATCH] Make PngChunk an immutable struct --- src/ImageSharp/Formats/Png/PngChunk.cs | 30 +++++-- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 95 ++++++++++---------- 2 files changed, 68 insertions(+), 57 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index b944b43a34..399bc95c92 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -8,11 +8,14 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Stores header information about a chunk. /// - internal sealed class PngChunk + internal readonly struct PngChunk { - public PngChunk(int length) + public PngChunk(int length, string type, IManagedByteBuffer data = null, uint crc = default) { this.Length = length; + this.Type = type; + this.Data = data; + this.Crc = crc; } /// @@ -24,21 +27,30 @@ namespace SixLabors.ImageSharp.Formats.Png public int Length { get; } /// - /// Gets or sets the chunk type as string with 4 chars. + /// Gets the chunk type as string with 4 chars. /// - public string Type { get; set; } + public string Type { get; } /// - /// Gets or sets the data bytes appropriate to the chunk type, if any. - /// This field can be of zero length. + /// Gets the data bytes appropriate to the chunk type, if any. + /// This field can be of zero length or null. /// - public IManagedByteBuffer Data { get; set; } + public IManagedByteBuffer Data { get; } /// - /// Gets or sets a CRC (Cyclic Redundancy Check) calculated on the preceding bytes in the chunk, + /// Gets a CRC (Cyclic Redundancy Check) calculated on the preceding bytes in the chunk, /// including the chunk type code and chunk data fields, but not including the length field. /// The CRC is always present, even for chunks containing no data /// - public uint Crc { get; set; } + public uint Crc { get; } + + /// + /// 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; } } diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index b3904c0a37..f4b045759e 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -262,12 +262,7 @@ namespace SixLabors.ImageSharp.Formats.Png } finally { - // Data is rented in ReadChunkData() - if (chunk.Data != null) - { - chunk.Data.Dispose(); - chunk.Data = null; - } + chunk.Data?.Dispose(); // Data is rented in ReadChunkData() } } } @@ -383,20 +378,6 @@ namespace SixLabors.ImageSharp.Formats.Png return result; } - /// - /// Returns a value indicating whether the given chunk is critical to decoding - /// - /// The chunk - /// The - private static bool IsCriticalChunk(PngChunk chunk) - { - return - chunk.Type == PngChunkTypes.Header || - chunk.Type == PngChunkTypes.Palette || - chunk.Type == PngChunkTypes.Data || - chunk.Type == PngChunkTypes.End; - } - /// /// Reads an integer value from 2 consecutive bytes in LSB order /// @@ -1217,38 +1198,63 @@ namespace SixLabors.ImageSharp.Formats.Png return false; } - chunk = new PngChunk(length); - - if (chunk.Length < 0 || chunk.Length > this.currentStream.Length - this.currentStream.Position) + while (length < 0 || length > (this.currentStream.Length - this.currentStream.Position)) { // Not a valid chunk so we skip back all but one of the four bytes we have just read. // That lets us read one byte at a time until we reach a known chunk. this.currentStream.Position -= 3; - return true; + length = this.ReadChunkLength(); + + if (length == -1) + { + chunk = default; + + return false; + } } - this.ReadChunkType(chunk); + string type = this.ReadChunkType(); - if (chunk.Type == PngChunkTypes.Data) + // NOTE: Handling the chunk data is the responsible of the caller + // It is currently either skipped (in identification) or read during decoding + if (type == PngChunkTypes.Data) { + chunk = new PngChunk(length, type); + return true; } - this.ReadChunkData(chunk); - this.ReadChunkCrc(chunk); + chunk = new PngChunk( + length: length, + type: type, + data: this.ReadChunkData(length), + crc: this.ReadChunkCrc()); + + this.ValidateChunk(chunk); return true; } + private void ValidateChunk(in PngChunk chunk) + { + this.crc.Reset(); + this.crc.Update(this.chunkTypeBuffer); + this.crc.Update(new ReadOnlySpan(chunk.Data.Array, 0, chunk.Length)); + + if (this.crc.Value != chunk.Crc && chunk.IsCritical) + { + throw new ImageFormatException($"CRC Error. PNG {chunk.Type} chunk is corrupt!"); + } + } + /// /// Reads the cycle redundancy chunk from the data. /// - /// The chunk. /// /// Thrown if the input stream is not valid or corrupt. /// - private void ReadChunkCrc(PngChunk chunk) + private uint ReadChunkCrc() { int numBytes = this.currentStream.Read(this.crcBuffer, 0, 4); @@ -1257,22 +1263,13 @@ namespace SixLabors.ImageSharp.Formats.Png throw new ImageFormatException("Image stream is not valid!"); } - chunk.Crc = BinaryPrimitives.ReadUInt32BigEndian(this.crcBuffer); - - this.crc.Reset(); - this.crc.Update(this.chunkTypeBuffer); - this.crc.Update(new ReadOnlySpan(chunk.Data.Array, 0, chunk.Length)); - - if (this.crc.Value != chunk.Crc && IsCriticalChunk(chunk)) - { - throw new ImageFormatException($"CRC Error. PNG {chunk.Type} chunk is corrupt!"); - } + return BinaryPrimitives.ReadUInt32BigEndian(this.crcBuffer); } /// /// Skips the chunk data and the cycle redundancy chunk read from the data. /// - private void SkipChunkDataAndCrc(PngChunk chunk) + private void SkipChunkDataAndCrc(in PngChunk chunk) { this.currentStream.Skip(chunk.Length); this.currentStream.Skip(4); @@ -1281,22 +1278,24 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Reads the chunk data from the stream. /// - /// The chunk. - private void ReadChunkData(PngChunk chunk) + /// The length of the chunk data to read. + private IManagedByteBuffer ReadChunkData(int length) { // We rent the buffer here to return it afterwards in Decode() - chunk.Data = this.configuration.MemoryManager.AllocateCleanManagedByteBuffer(chunk.Length); - this.currentStream.Read(chunk.Data.Array, 0, chunk.Length); + IManagedByteBuffer buffer = this.configuration.MemoryManager.AllocateCleanManagedByteBuffer(length); + + this.currentStream.Read(buffer.Array, 0, length); + + return buffer; } /// /// Identifies the chunk type from the chunk. /// - /// The chunk. /// /// Thrown if the input stream is not valid. /// - private void ReadChunkType(PngChunk chunk) + private string ReadChunkType() { int numBytes = this.currentStream.Read(this.chunkTypeBuffer, 0, 4); if (numBytes >= 1 && numBytes <= 3) @@ -1309,7 +1308,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.chars[2] = (char)this.chunkTypeBuffer[2]; this.chars[3] = (char)this.chunkTypeBuffer[3]; - chunk.Type = new string(this.chars); + return new string(this.chars); } ///