diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index c75f9465a..1fee4a837 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using SixLabors.Memory; @@ -10,12 +10,11 @@ namespace SixLabors.ImageSharp.Formats.Png /// internal readonly struct PngChunk { - public PngChunk(int length, PngChunkType type, IManagedByteBuffer data = null, uint crc = 0) + public PngChunk(int length, PngChunkType type, IManagedByteBuffer data = null) { this.Length = length; this.Type = type; this.Data = data; - this.Crc = crc; } /// @@ -38,20 +37,12 @@ namespace SixLabors.ImageSharp.Formats.Png /// public IManagedByteBuffer Data { get; } - /// - /// 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; } - /// /// Gets a value indicating whether the given chunk is critical to decoding /// public bool IsCritical => this.Type == PngChunkType.Header || this.Type == PngChunkType.Palette || - this.Type == PngChunkType.Data || - this.Type == PngChunkType.End; + this.Type == PngChunkType.Data; } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 037f648f0..b24a5eabd 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -221,7 +221,7 @@ namespace SixLabors.ImageSharp.Formats.Png if (image is null) { - throw new ImageFormatException("PNG Image does not contain a data chunk"); + PngThrowHelper.ThrowNoData(); } return image; @@ -285,7 +285,7 @@ namespace SixLabors.ImageSharp.Formats.Png if (this.header.Width == 0 && this.header.Height == 0) { - throw new ImageFormatException("PNG Image does not contain a header chunk"); + PngThrowHelper.ThrowNoHeader(); } return new ImageInfo(new PixelTypeInfo(this.CalculateBitsPerPixel()), this.header.Width, this.header.Height, metadata); @@ -407,7 +407,8 @@ namespace SixLabors.ImageSharp.Formats.Png case PngColorType.RgbWithAlpha: return this.header.BitDepth * 4; default: - throw new NotSupportedException("Unsupported PNG color type"); + PngThrowHelper.ThrowNotSupportedColor(); + return -1; } } @@ -528,7 +529,8 @@ namespace SixLabors.ImageSharp.Formats.Png break; default: - throw new ImageFormatException("Unknown filter type."); + PngThrowHelper.ThrowUnknownFilter(); + break; } this.ProcessDefilteredScanline(scanlineSpan, image, pngMetadata); @@ -601,7 +603,8 @@ namespace SixLabors.ImageSharp.Formats.Png break; default: - throw new ImageFormatException("Unknown filter type."); + PngThrowHelper.ThrowUnknownFilter(); + break; } Span rowSpan = image.GetPixelRowSpan(this.currentRow); @@ -1119,13 +1122,9 @@ namespace SixLabors.ImageSharp.Formats.Png chunk = new PngChunk( length: length, type: type, - data: this.ReadChunkData(length), - crc: this.ReadChunkCrc()); + data: this.ReadChunkData(length)); - if (chunk.IsCritical) - { - this.ValidateChunk(chunk); - } + this.ValidateChunk(chunk); return true; } @@ -1136,6 +1135,11 @@ namespace SixLabors.ImageSharp.Formats.Png /// The . private void ValidateChunk(in PngChunk chunk) { + if (!chunk.IsCritical) + { + return; + } + Span chunkType = stackalloc byte[4]; BinaryPrimitives.WriteUInt32BigEndian(chunkType, (uint)chunk.Type); @@ -1144,31 +1148,34 @@ namespace SixLabors.ImageSharp.Formats.Png this.crc.Update(chunkType); this.crc.Update(chunk.Data.GetSpan()); - if (this.crc.Value != chunk.Crc) + uint crc = this.ReadChunkCrc(); + if (this.crc.Value != crc) { string chunkTypeName = Encoding.ASCII.GetString(chunkType); - - throw new ImageFormatException($"CRC Error. PNG {chunkTypeName} chunk is corrupt!"); + PngThrowHelper.ThrowInvalidChunkCrc(chunkTypeName); } } /// /// Reads the cycle redundancy chunk from the data. /// - /// - /// Thrown if the input stream is not valid or corrupt. - /// + [MethodImpl(InliningOptions.ShortMethod)] private uint ReadChunkCrc() { - return this.currentStream.Read(this.buffer, 0, 4) == 4 - ? BinaryPrimitives.ReadUInt32BigEndian(this.buffer) - : throw new ImageFormatException("Image stream is not valid!"); + uint crc = 0; + if (this.currentStream.Read(this.buffer, 0, 4) == 4) + { + crc = BinaryPrimitives.ReadUInt32BigEndian(this.buffer); + } + + return crc; } /// /// Skips the chunk data and the cycle redundancy chunk read from the data. /// /// The image format chunk. + [MethodImpl(InliningOptions.ShortMethod)] private void SkipChunkDataAndCrc(in PngChunk chunk) { this.currentStream.Skip(chunk.Length); @@ -1179,6 +1186,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// Reads the chunk data from the stream. /// /// The length of the chunk data to read. + [MethodImpl(InliningOptions.ShortMethod)] private IManagedByteBuffer ReadChunkData(int length) { // We rent the buffer here to return it afterwards in Decode() @@ -1195,11 +1203,20 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Thrown if the input stream is not valid. /// + [MethodImpl(InliningOptions.ShortMethod)] private PngChunkType ReadChunkType() { - return this.currentStream.Read(this.buffer, 0, 4) == 4 - ? (PngChunkType)BinaryPrimitives.ReadUInt32BigEndian(this.buffer) - : throw new ImageFormatException("Invalid PNG data."); + if (this.currentStream.Read(this.buffer, 0, 4) == 4) + { + return (PngChunkType)BinaryPrimitives.ReadUInt32BigEndian(this.buffer); + } + else + { + PngThrowHelper.ThrowInvalidChunkType(); + + // The IDE cannot detect the throw here. + return default; + } } /// @@ -1208,6 +1225,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Whether the length was read. /// + [MethodImpl(InliningOptions.ShortMethod)] private bool TryReadChunkLength(out int result) { if (this.currentStream.Read(this.buffer, 0, 4) == 4) diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs new file mode 100644 index 000000000..00b40c50b --- /dev/null +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -0,0 +1,33 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; + +namespace SixLabors.ImageSharp.Formats.Png +{ + /// + /// Cold path optimizations for throwing png format based exceptions. + /// + internal static class PngThrowHelper + { + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowNoHeader() => throw new ImageFormatException("PNG Image does not contain a header chunk"); + + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowNoData() => throw new ImageFormatException("PNG Image does not contain a data chunk"); + + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowInvalidChunkType() => throw new ImageFormatException("Invalid PNG data."); + + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowInvalidChunkCrc(string chunkTypeName) => throw new ImageFormatException($"CRC Error. PNG {chunkTypeName} chunk is corrupt!"); + + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowNotSupportedColor() => new NotSupportedException("Unsupported PNG color type"); + + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowUnknownFilter() => throw new ImageFormatException("Unknown filter type."); + } +} diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs index e976d5a76..660d5b724 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.Chunks.cs @@ -54,7 +54,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png [InlineData((uint)PngChunkType.Header)] // IHDR [InlineData((uint)PngChunkType.Palette)] // PLTE // [InlineData(PngChunkTypes.Data)] //TODO: Figure out how to test this - [InlineData((uint)PngChunkType.End)] // IEND public void Decode_IncorrectCRCForCriticalChunk_ExceptionIsThrown(uint chunkType) { string chunkName = GetChunkTypeName(chunkType); @@ -74,26 +73,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png } } - [Theory] - [InlineData((uint)PngChunkType.Gamma)] // gAMA - [InlineData((uint)PngChunkType.Transparency)] // tRNS - [InlineData((uint)PngChunkType.Physical)] // pHYs: 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(uint chunkType) - { - string chunkName = GetChunkTypeName(chunkType); - - using (var memStream = new MemoryStream()) - { - WriteHeaderChunk(memStream); - WriteChunk(memStream, chunkName); - WriteDataChunk(memStream); - - var decoder = new PngDecoder(); - decoder.Decode(null, memStream); - } - } - private static string GetChunkTypeName(uint value) { var data = new byte[4]; diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index e064c0fb0..bdd84038e 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -4,11 +4,11 @@ // ReSharper disable InconsistentNaming using System.IO; - +using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; - +using SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs; using Xunit; namespace SixLabors.ImageSharp.Tests.Formats.Png @@ -42,6 +42,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png TestImages.Png.Bad.ZlibOverflow, TestImages.Png.Bad.ZlibOverflow2, TestImages.Png.Bad.ZlibZtxtBadHeader, + TestImages.Png.Bad.Issue1047_BadEndChunk }; public static readonly string[] TestImages48Bpp = @@ -90,7 +91,19 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png using (Image image = provider.GetImage(new PngDecoder())) { image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); + + // We don't have another x-plat reference decoder that can be compared for this image. + if (provider.Utility.SourceFileOrDescription == TestImages.Png.Bad.Issue1047_BadEndChunk) + { + if (TestEnvironment.IsWindows) + { + image.CompareToOriginal(provider, ImageComparer.Exact, (IImageDecoder)SystemDrawingReferenceDecoder.Instance); + } + } + else + { + image.CompareToOriginal(provider, ImageComparer.Exact); + } } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 177749869..d19dbe834 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -99,6 +99,7 @@ namespace SixLabors.ImageSharp.Tests public const string ZlibOverflow = "Png/zlib-overflow.png"; public const string ZlibOverflow2 = "Png/zlib-overflow2.png"; public const string ZlibZtxtBadHeader = "Png/zlib-ztxt-bad-header.png"; + public const string Issue1047_BadEndChunk = "Png/issues/Issue_1047.png"; } public static readonly string[] All = diff --git a/tests/Images/Input/Png/issues/Issue_1047.png b/tests/Images/Input/Png/issues/Issue_1047.png new file mode 100644 index 000000000..0f85dc53c Binary files /dev/null and b/tests/Images/Input/Png/issues/Issue_1047.png differ