From c99c83ae61a7aa1fdeabfa6fb18b49fe4af77a97 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 29 Nov 2023 19:44:10 +1000 Subject: [PATCH] Create fine-grained options for CRC handling. --- src/ImageSharp/Formats/Png/PngChunk.cs | 14 +++++---- .../Formats/Png/PngCrcChunkHandling.cs | 30 +++++++++++++++++++ src/ImageSharp/Formats/Png/PngDecoderCore.cs | 18 +++++------ .../Formats/Png/PngDecoderOptions.cs | 5 ++-- .../Formats/Png/PngDecoderTests.cs | 2 +- 5 files changed, 49 insertions(+), 20 deletions(-) create mode 100644 src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index e5fa5fbb7..6ec0df9ad 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -41,9 +41,13 @@ internal readonly struct PngChunk /// /// Gets a value indicating whether the given chunk is critical to decoding /// - public bool IsCritical => - this.Type is PngChunkType.Header or - PngChunkType.Palette or - PngChunkType.Data or - PngChunkType.FrameData; + /// The chunk CRC handling behavior. + public bool IsCritical(PngCrcChunkHandling handling) + => handling switch + { + PngCrcChunkHandling.IgnoreNone => true, + PngCrcChunkHandling.IgnoreNonCritical => this.Type is PngChunkType.Header or PngChunkType.Palette or PngChunkType.Data or PngChunkType.FrameData, + PngCrcChunkHandling.IgnoreData => this.Type is PngChunkType.Header or PngChunkType.Palette, + _ => false, + }; } diff --git a/src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs b/src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs new file mode 100644 index 000000000..264d737fd --- /dev/null +++ b/src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs @@ -0,0 +1,30 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +namespace SixLabors.ImageSharp.Formats.Png; + +/// +/// Specifies how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG. +/// +public enum PngCrcChunkHandling +{ + /// + /// Do not ignore any CRC chunk errors. + /// + IgnoreNone, + + /// + /// Ignore CRC errors in non critical chunks. + /// + IgnoreNonCritical, + + /// + /// Ignore CRC errors in data chunks. + /// + IgnoreData, + + /// + /// Ignore CRC errors in all chunks. + /// + IgnoreAll +} diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 355a0bfcf..4edbe50c7 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -115,9 +115,9 @@ internal sealed class PngDecoderCore : IImageDecoderInternals private PngChunk? nextChunk; /// - /// If true, ADLER32 checksum in the IDAT chunk as well as the chunk CRCs will be ignored. + /// How to handle CRC errors. /// - private bool ignoreCrcErrors; + private readonly PngCrcChunkHandling pngCrcChunkHandling; /// /// Initializes a new instance of the class. @@ -130,7 +130,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals this.maxFrames = options.GeneralOptions.MaxFrames; this.skipMetadata = options.GeneralOptions.SkipMetadata; this.memoryAllocator = this.configuration.MemoryAllocator; - this.ignoreCrcErrors = options.IgnoreCrcCheck; + this.pngCrcChunkHandling = options.PngCrcChunkHandling; } internal PngDecoderCore(PngDecoderOptions options, bool colorMetadataOnly) @@ -141,7 +141,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals this.skipMetadata = true; this.configuration = options.GeneralOptions.Configuration; this.memoryAllocator = this.configuration.MemoryAllocator; - this.ignoreCrcErrors = options.IgnoreCrcCheck; + this.pngCrcChunkHandling = options.PngCrcChunkHandling; } /// @@ -1797,11 +1797,8 @@ internal sealed class PngDecoderCore : IImageDecoderInternals type: type, data: this.ReadChunkData(length)); - if (!this.ignoreCrcErrors) - { - this.ValidateChunk(chunk, buffer); - } - + this.ValidateChunk(chunk, buffer); + // Restore the stream position for IDAT and fdAT chunks, because it will be decoded later and // was only read to verifying the CRC is correct. if (type is PngChunkType.Data or PngChunkType.FrameData) @@ -1820,8 +1817,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals private void ValidateChunk(in PngChunk chunk, Span buffer) { uint inputCrc = this.ReadChunkCrc(buffer); - - if (chunk.IsCritical) + if (chunk.IsCritical(this.pngCrcChunkHandling)) { Span chunkType = stackalloc byte[4]; BinaryPrimitives.WriteUInt32BigEndian(chunkType, (uint)chunk.Type); diff --git a/src/ImageSharp/Formats/Png/PngDecoderOptions.cs b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs index c952a18ef..ab6ba4770 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderOptions.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs @@ -12,8 +12,7 @@ public sealed class PngDecoderOptions : ISpecializedDecoderOptions public DecoderOptions GeneralOptions { get; init; } = new DecoderOptions(); /// - /// If true, ADLER32 checksum in the IDAT chunk as well as the chunk CRCs will be ignored. - /// Similar to PNG_CRC_QUIET_USE in libpng. + /// Gets a value indicating how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG. /// - public bool IgnoreCrcCheck { get; init; } + public PngCrcChunkHandling PngCrcChunkHandling { get; init; } = PngCrcChunkHandling.IgnoreNonCritical; } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 5351af4fc..cbb625bdf 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -475,7 +475,7 @@ public partial class PngDecoderTests public void Decode_InvalidDataChunkCrc_IgnoreCrcErrors(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using Image image = provider.GetImage(PngDecoder.Instance, new PngDecoderOptions() { IgnoreCrcCheck = true }); + using Image image = provider.GetImage(PngDecoder.Instance, new PngDecoderOptions() { PngCrcChunkHandling = PngCrcChunkHandling.IgnoreData }); image.DebugSave(provider); image.CompareToOriginal(provider, new MagickReferenceDecoder(false));