From 72b80132416a88873f5f836b32d0ef718cb1a4f9 Mon Sep 17 00:00:00 2001 From: Sven Claesson Date: Fri, 10 Nov 2023 13:42:25 +0100 Subject: [PATCH 1/2] refactor PngDecoder to SpecializedImageDecoder --- src/ImageSharp/Formats/Png/PngDecoder.cs | 17 ++++++---- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 32 ++++++++++++------- .../Formats/Png/PngDecoderOptions.cs | 19 +++++++++++ .../Formats/Png/PngDecoderTests.cs | 11 +++++++ .../ReferenceCodecs/MagickReferenceDecoder.cs | 5 +++ 5 files changed, 66 insertions(+), 18 deletions(-) create mode 100644 src/ImageSharp/Formats/Png/PngDecoderOptions.cs diff --git a/src/ImageSharp/Formats/Png/PngDecoder.cs b/src/ImageSharp/Formats/Png/PngDecoder.cs index d226451389..4a7ba3f961 100644 --- a/src/ImageSharp/Formats/Png/PngDecoder.cs +++ b/src/ImageSharp/Formats/Png/PngDecoder.cs @@ -8,7 +8,7 @@ namespace SixLabors.ImageSharp.Formats.Png; /// /// Decoder for generating an image out of a png encoded stream. /// -public sealed class PngDecoder : ImageDecoder +public sealed class PngDecoder : SpecializedImageDecoder { private PngDecoder() { @@ -25,31 +25,31 @@ protected override ImageInfo Identify(DecoderOptions options, Stream stream, Can Guard.NotNull(options, nameof(options)); Guard.NotNull(stream, nameof(stream)); - return new PngDecoderCore(options).Identify(options.Configuration, stream, cancellationToken); + return new PngDecoderCore(new PngDecoderOptions() { GeneralOptions = options }).Identify(options.Configuration, stream, cancellationToken); } /// - protected override Image Decode(DecoderOptions options, Stream stream, CancellationToken cancellationToken) + protected override Image Decode(PngDecoderOptions options, Stream stream, CancellationToken cancellationToken) { Guard.NotNull(options, nameof(options)); Guard.NotNull(stream, nameof(stream)); PngDecoderCore decoder = new(options); - Image image = decoder.Decode(options.Configuration, stream, cancellationToken); + Image image = decoder.Decode(options.GeneralOptions.Configuration, stream, cancellationToken); - ScaleToTargetSize(options, image); + ScaleToTargetSize(options.GeneralOptions, image); return image; } /// - protected override Image Decode(DecoderOptions options, Stream stream, CancellationToken cancellationToken) + protected override Image Decode(PngDecoderOptions options, Stream stream, CancellationToken cancellationToken) { Guard.NotNull(options, nameof(options)); Guard.NotNull(stream, nameof(stream)); PngDecoderCore decoder = new(options, true); - ImageInfo info = decoder.Identify(options.Configuration, stream, cancellationToken); + ImageInfo info = decoder.Identify(options.GeneralOptions.Configuration, stream, cancellationToken); stream.Position = 0; PngMetadata meta = info.Metadata.GetPngMetadata(); @@ -99,4 +99,7 @@ protected override Image Decode(DecoderOptions options, Stream stream, Cancellat return this.Decode(options, stream, cancellationToken); } } + + /// + protected override PngDecoderOptions CreateDefaultSpecializedOptions(DecoderOptions options) => new PngDecoderOptions() { GeneralOptions = options }; } diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index d8305a3f57..0957a7bd53 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -113,27 +113,34 @@ 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. + /// + private bool ignoreCrcErrors; + /// /// Initializes a new instance of the class. /// /// The decoder options. - public PngDecoderCore(DecoderOptions options) + public PngDecoderCore(PngDecoderOptions options) { - this.Options = options; - this.configuration = options.Configuration; - this.maxFrames = options.MaxFrames; - this.skipMetadata = options.SkipMetadata; + this.Options = options.GeneralOptions; + this.configuration = options.GeneralOptions.Configuration; + this.maxFrames = options.GeneralOptions.MaxFrames; + this.skipMetadata = options.GeneralOptions.SkipMetadata; this.memoryAllocator = this.configuration.MemoryAllocator; + this.ignoreCrcErrors = options.IgnoreCrcCheck; } - internal PngDecoderCore(DecoderOptions options, bool colorMetadataOnly) + internal PngDecoderCore(PngDecoderOptions options, bool colorMetadataOnly) { - this.Options = options; + this.Options = options.GeneralOptions; this.colorMetadataOnly = colorMetadataOnly; - this.maxFrames = options.MaxFrames; + this.maxFrames = options.GeneralOptions.MaxFrames; this.skipMetadata = true; - this.configuration = options.Configuration; + this.configuration = options.GeneralOptions.Configuration; this.memoryAllocator = this.configuration.MemoryAllocator; + this.ignoreCrcErrors = options.IgnoreCrcCheck; } /// @@ -1789,8 +1796,11 @@ private bool TryReadChunk(Span buffer, out PngChunk chunk) type: type, data: this.ReadChunkData(length)); - this.ValidateChunk(chunk, buffer); - + if (!this.ignoreCrcErrors) + { + 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) diff --git a/src/ImageSharp/Formats/Png/PngDecoderOptions.cs b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs new file mode 100644 index 0000000000..c952a18ef2 --- /dev/null +++ b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs @@ -0,0 +1,19 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +namespace SixLabors.ImageSharp.Formats.Png; + +/// +/// Configuration options for decoding png images. +/// +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. + /// + public bool IgnoreCrcCheck { get; init; } +} diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 2e11093db6..a2925d254e 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -470,6 +470,17 @@ public void Decode_InvalidDataChunkCrc_ThrowsException(TestImageProvider Assert.Contains("CRC Error. PNG IDAT chunk is corrupt!", ex.Message); } + [Theory] + [WithFile(TestImages.Png.Bad.WrongCrcDataChunk, PixelTypes.Rgba32)] + public void Decode_InvalidDataChunkCrc_IgnoreCrcErrors(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder.Instance, new PngDecoderOptions() { IgnoreCrcCheck = true }); + + image.DebugSave(provider); + image.CompareToOriginal(provider, new MagickReferenceDecoder(false)); + } + // https://github.com/SixLabors/ImageSharp/issues/1014 [Theory] [WithFileCollection(nameof(TestImagesIssue1014), PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs index ae09c4f3f2..80b5536ebd 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs @@ -31,12 +31,17 @@ protected override Image Decode(DecoderOptions options, Stream s { IgnoreFileSize = !this.validate }; + PngReadDefines pngReadDefines = new() + { + IgnoreCrc = !this.validate + }; MagickReadSettings settings = new() { FrameCount = (int)options.MaxFrames }; settings.SetDefines(bmpReadDefines); + settings.SetDefines(pngReadDefines); using MagickImageCollection magickImageCollection = new(stream, settings); List> framesList = new(); From 0c30a08b8152c2509cc79e86aa6e0606ac864490 Mon Sep 17 00:00:00 2001 From: Sven Claesson Date: Fri, 10 Nov 2023 14:12:24 +0100 Subject: [PATCH 2/2] fix old copy paste issue --- tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs b/tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs index 74c7fc1d09..324bd4783a 100644 --- a/tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs +++ b/tests/ImageSharp.Tests/Formats/ImageFormatManagerTests.cs @@ -43,7 +43,7 @@ public void IfAutoLoadWellKnownFormatsIsTrueAllFormatsAreLoaded() Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); - Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); + Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count()); Assert.Equal(1, this.DefaultFormatsManager.ImageDecoders.Select(item => item.Value).OfType().Count());