From 61c30de89e277a321d0a8a2dd189256cd1503a5d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 30 Nov 2021 11:56:56 +1100 Subject: [PATCH 01/11] Stub out approach --- src/ImageSharp/Formats/Png/PngDecoder.cs | 17 +++++++- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 42 +++++++++++--------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoder.cs b/src/ImageSharp/Formats/Png/PngDecoder.cs index 479c24b7e8..ea898b0506 100644 --- a/src/ImageSharp/Formats/Png/PngDecoder.cs +++ b/src/ImageSharp/Formats/Png/PngDecoder.cs @@ -20,12 +20,25 @@ namespace SixLabors.ImageSharp.Formats.Png public Image Decode(Configuration configuration, Stream stream) where TPixel : unmanaged, IPixel { - var decoder = new PngDecoderCore(configuration, this); + PngDecoderCore decoder = new(configuration, this); return decoder.Decode(configuration, stream); } /// - public Image Decode(Configuration configuration, Stream stream) => this.Decode(configuration, stream); + public Image Decode(Configuration configuration, Stream stream) + { + PngDecoderCore decoder = new(configuration, true); + IImageInfo info = decoder.Identify(configuration, stream); + stream.Position = 0; + + switch (info.Metadata.GetPngMetadata().ColorType) + { + default: + break; + } + + return this.Decode(configuration, stream); + } /// public Task> DecodeAsync(Configuration configuration, Stream stream, CancellationToken cancellationToken) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index cf3cd7eb14..e741db72c6 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -124,6 +124,13 @@ namespace SixLabors.ImageSharp.Formats.Png this.ignoreMetadata = options.IgnoreMetadata; } + internal PngDecoderCore(Configuration configuration, bool ignoreMetadata) + { + this.Configuration = configuration ?? Configuration.Default; + this.memoryAllocator = this.Configuration.MemoryAllocator; + this.ignoreMetadata = ignoreMetadata; + } + /// public Configuration Configuration { get; } @@ -168,12 +175,12 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngChunkType.Palette: - var pal = new byte[chunk.Length]; + byte[] pal = new byte[chunk.Length]; chunk.Data.GetSpan().CopyTo(pal); this.palette = pal; break; case PngChunkType.Transparency: - var alpha = new byte[chunk.Length]; + byte[] alpha = new byte[chunk.Length]; chunk.Data.GetSpan().CopyTo(alpha); this.paletteAlpha = alpha; this.AssignTransparentMarkers(alpha, pngMetadata); @@ -190,7 +197,7 @@ namespace SixLabors.ImageSharp.Formats.Png case PngChunkType.Exif: if (!this.ignoreMetadata) { - var exifData = new byte[chunk.Length]; + byte[] exifData = new byte[chunk.Length]; chunk.Data.GetSpan().CopyTo(exifData); metadata.ExifProfile = new ExifProfile(exifData); } @@ -263,7 +270,7 @@ namespace SixLabors.ImageSharp.Formats.Png case PngChunkType.Exif: if (!this.ignoreMetadata) { - var exifData = new byte[chunk.Length]; + byte[] exifData = new byte[chunk.Length]; chunk.Data.GetSpan().CopyTo(exifData); metadata.ExifProfile = new ExifProfile(exifData); } @@ -364,11 +371,10 @@ namespace SixLabors.ImageSharp.Formats.Png /// The metadata to read to. /// The data containing physical data. private void ReadGammaChunk(PngMetadata pngMetadata, ReadOnlySpan data) - { + // The value is encoded as a 4-byte unsigned integer, representing gamma times 100000. // For example, a gamma of 1/2.2 would be stored as 45455. - pngMetadata.Gamma = BinaryPrimitives.ReadUInt32BigEndian(data) / 100_000F; - } + => pngMetadata.Gamma = BinaryPrimitives.ReadUInt32BigEndian(data) / 100_000F; /// /// Initializes the image and various buffers needed for processing @@ -477,19 +483,17 @@ namespace SixLabors.ImageSharp.Formats.Png private void ReadScanlines(PngChunk chunk, ImageFrame image, PngMetadata pngMetadata) where TPixel : unmanaged, IPixel { - using (var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk)) - { - deframeStream.AllocateNewBytes(chunk.Length, true); - DeflateStream dataStream = deframeStream.CompressedStream; + using var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk); + deframeStream.AllocateNewBytes(chunk.Length, true); + DeflateStream dataStream = deframeStream.CompressedStream; - if (this.header.InterlaceMethod == PngInterlaceMode.Adam7) - { - this.DecodeInterlacedPixelData(dataStream, image, pngMetadata); - } - else - { - this.DecodePixelData(dataStream, image, pngMetadata); - } + if (this.header.InterlaceMethod == PngInterlaceMode.Adam7) + { + this.DecodeInterlacedPixelData(dataStream, image, pngMetadata); + } + else + { + this.DecodePixelData(dataStream, image, pngMetadata); } } From 65bd0de7a21bb554f86f11009697e35957d19619 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 30 Nov 2021 17:44:23 +1100 Subject: [PATCH 02/11] Update PngDecoder.cs --- src/ImageSharp/Formats/Png/PngDecoder.cs | 63 ++++++++++++++++++++---- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoder.cs b/src/ImageSharp/Formats/Png/PngDecoder.cs index ea898b0506..5637d5c7bf 100644 --- a/src/ImageSharp/Formats/Png/PngDecoder.cs +++ b/src/ImageSharp/Formats/Png/PngDecoder.cs @@ -31,39 +31,82 @@ namespace SixLabors.ImageSharp.Formats.Png IImageInfo info = decoder.Identify(configuration, stream); stream.Position = 0; - switch (info.Metadata.GetPngMetadata().ColorType) + PngMetadata meta = info.Metadata.GetPngMetadata(); + PngColorType color = meta.ColorType.GetValueOrDefault(); + PngBitDepth bits = meta.BitDepth.GetValueOrDefault(); + return color switch { - default: - break; - } + PngColorType.Grayscale => (bits == PngBitDepth.Bit16) + ? this.Decode(configuration, stream) + : this.Decode(configuration, stream), - return this.Decode(configuration, stream); + PngColorType.Rgb => this.Decode(configuration, stream), + + PngColorType.Palette => this.Decode(configuration, stream), + + PngColorType.GrayscaleWithAlpha => (bits == PngBitDepth.Bit16) + ? this.Decode(configuration, stream) + : this.Decode(configuration, stream), + + PngColorType.RgbWithAlpha => (bits == PngBitDepth.Bit16) + ? this.Decode(configuration, stream) + : this.Decode(configuration, stream), + + _ => this.Decode(configuration, stream), + }; } /// public Task> DecodeAsync(Configuration configuration, Stream stream, CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - var decoder = new PngDecoderCore(configuration, this); + PngDecoderCore decoder = new(configuration, this); return decoder.DecodeAsync(configuration, stream, cancellationToken); } /// public async Task DecodeAsync(Configuration configuration, Stream stream, CancellationToken cancellationToken) - => await this.DecodeAsync(configuration, stream, cancellationToken) - .ConfigureAwait(false); + { + PngDecoderCore decoder = new(configuration, true); + IImageInfo info = await decoder.IdentifyAsync(configuration, stream, cancellationToken).ConfigureAwait(false); + stream.Position = 0; + + PngMetadata meta = info.Metadata.GetPngMetadata(); + PngColorType color = meta.ColorType.GetValueOrDefault(); + PngBitDepth bits = meta.BitDepth.GetValueOrDefault(); + return color switch + { + PngColorType.Grayscale => (bits == PngBitDepth.Bit16) + ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) + : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), + + PngColorType.Rgb => await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), + + PngColorType.Palette => await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), + + PngColorType.GrayscaleWithAlpha => (bits == PngBitDepth.Bit16) + ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) + : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), + + PngColorType.RgbWithAlpha => (bits == PngBitDepth.Bit16) + ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) + : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), + + _ => await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), + }; + } /// public IImageInfo Identify(Configuration configuration, Stream stream) { - var decoder = new PngDecoderCore(configuration, this); + PngDecoderCore decoder = new(configuration, this); return decoder.Identify(configuration, stream); } /// public Task IdentifyAsync(Configuration configuration, Stream stream, CancellationToken cancellationToken) { - var decoder = new PngDecoderCore(configuration, this); + PngDecoderCore decoder = new(configuration, this); return decoder.IdentifyAsync(configuration, stream, cancellationToken); } } From d7032fe6ee7d43a4a6415a7a4d17f84afdb55e4f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 1 Dec 2021 23:05:21 +1100 Subject: [PATCH 03/11] Update src/ImageSharp/Formats/Png/PngDecoderCore.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index e741db72c6..333b00f23d 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -374,7 +374,7 @@ namespace SixLabors.ImageSharp.Formats.Png // The value is encoded as a 4-byte unsigned integer, representing gamma times 100000. // For example, a gamma of 1/2.2 would be stored as 45455. - => pngMetadata.Gamma = BinaryPrimitives.ReadUInt32BigEndian(data) / 100_000F; + => pngMetadata.Gamma = BinaryPrimitives.ReadUInt32BigEndian(data) * 1e-5F; /// /// Initializes the image and various buffers needed for processing From 8fd9b7224cff177789f30ba14c2235f91ca64f8d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 2 Dec 2021 00:37:05 +1100 Subject: [PATCH 04/11] Fix transparency chunk identification --- src/ImageSharp/Formats/Png/PngDecoder.cs | 120 +++++++++++++------ src/ImageSharp/Formats/Png/PngDecoderCore.cs | 6 + 2 files changed, 88 insertions(+), 38 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoder.cs b/src/ImageSharp/Formats/Png/PngDecoder.cs index 5637d5c7bf..04e70c51d0 100644 --- a/src/ImageSharp/Formats/Png/PngDecoder.cs +++ b/src/ImageSharp/Formats/Png/PngDecoder.cs @@ -34,26 +34,48 @@ namespace SixLabors.ImageSharp.Formats.Png PngMetadata meta = info.Metadata.GetPngMetadata(); PngColorType color = meta.ColorType.GetValueOrDefault(); PngBitDepth bits = meta.BitDepth.GetValueOrDefault(); - return color switch + switch (color) { - PngColorType.Grayscale => (bits == PngBitDepth.Bit16) - ? this.Decode(configuration, stream) - : this.Decode(configuration, stream), - - PngColorType.Rgb => this.Decode(configuration, stream), - - PngColorType.Palette => this.Decode(configuration, stream), - - PngColorType.GrayscaleWithAlpha => (bits == PngBitDepth.Bit16) - ? this.Decode(configuration, stream) - : this.Decode(configuration, stream), - - PngColorType.RgbWithAlpha => (bits == PngBitDepth.Bit16) - ? this.Decode(configuration, stream) - : this.Decode(configuration, stream), - - _ => this.Decode(configuration, stream), - }; + case PngColorType.Grayscale: + if (bits == PngBitDepth.Bit16) + { + return !meta.HasTransparency + ? this.Decode(configuration, stream) + : this.Decode(configuration, stream); + } + + return !meta.HasTransparency + ? this.Decode(configuration, stream) + : this.Decode(configuration, stream); + + case PngColorType.Rgb: + if (bits == PngBitDepth.Bit16) + { + return !meta.HasTransparency + ? this.Decode(configuration, stream) + : this.Decode(configuration, stream); + } + + return !meta.HasTransparency + ? this.Decode(configuration, stream) + : this.Decode(configuration, stream); + + case PngColorType.Palette: + return this.Decode(configuration, stream); + + case PngColorType.GrayscaleWithAlpha: + return (bits == PngBitDepth.Bit16) + ? this.Decode(configuration, stream) + : this.Decode(configuration, stream); + + case PngColorType.RgbWithAlpha: + return (bits == PngBitDepth.Bit16) + ? this.Decode(configuration, stream) + : this.Decode(configuration, stream); + + default: + return this.Decode(configuration, stream); + } } /// @@ -74,26 +96,48 @@ namespace SixLabors.ImageSharp.Formats.Png PngMetadata meta = info.Metadata.GetPngMetadata(); PngColorType color = meta.ColorType.GetValueOrDefault(); PngBitDepth bits = meta.BitDepth.GetValueOrDefault(); - return color switch + switch (color) { - PngColorType.Grayscale => (bits == PngBitDepth.Bit16) - ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) - : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), - - PngColorType.Rgb => await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), - - PngColorType.Palette => await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), - - PngColorType.GrayscaleWithAlpha => (bits == PngBitDepth.Bit16) - ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) - : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), - - PngColorType.RgbWithAlpha => (bits == PngBitDepth.Bit16) - ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) - : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), - - _ => await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false), - }; + case PngColorType.Grayscale: + if (bits == PngBitDepth.Bit16) + { + return !meta.HasTransparency + ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) + : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false); + } + + return !meta.HasTransparency + ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) + : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false); + + case PngColorType.Rgb: + if (bits == PngBitDepth.Bit16) + { + return !meta.HasTransparency + ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) + : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false); + } + + return !meta.HasTransparency + ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) + : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false); + + case PngColorType.Palette: + return await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false); + + case PngColorType.GrayscaleWithAlpha: + return (bits == PngBitDepth.Bit16) + ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) + : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false); + + case PngColorType.RgbWithAlpha: + return (bits == PngBitDepth.Bit16) + ? await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false) + : await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false); + + default: + return await this.DecodeAsync(configuration, stream, cancellationToken).ConfigureAwait(false); + } } /// diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 333b00f23d..ba737cb42b 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -258,6 +258,12 @@ namespace SixLabors.ImageSharp.Formats.Png case PngChunkType.Data: this.SkipChunkDataAndCrc(chunk); break; + case PngChunkType.Transparency: + byte[] alpha = new byte[chunk.Length]; + chunk.Data.GetSpan().CopyTo(alpha); + this.paletteAlpha = alpha; + this.AssignTransparentMarkers(alpha, pngMetadata); + break; case PngChunkType.Text: this.ReadTextChunk(pngMetadata, chunk.Data.GetSpan()); break; From eb68bb2d17732db0053804da46412f4b94839d11 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 2 Dec 2021 00:37:15 +1100 Subject: [PATCH 05/11] Update PngDecoderTests.cs --- .../Formats/Png/PngDecoderTests.cs | 227 +++++++++--------- 1 file changed, 111 insertions(+), 116 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 9fc4d03dda..82b5f718b2 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -1,7 +1,9 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; using System.IO; +using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; using SixLabors.ImageSharp.Formats.Png; @@ -20,9 +22,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png [Trait("Format", "Png")] public partial class PngDecoderTests { - private const PixelTypes PixelTypes = Tests.PixelTypes.Rgba32 | Tests.PixelTypes.RgbaVector | Tests.PixelTypes.Argb32; + private const PixelTypes TestPixelTypes = PixelTypes.Rgba32 | PixelTypes.RgbaVector | PixelTypes.Argb32; - private static PngDecoder PngDecoder => new PngDecoder(); + private static PngDecoder PngDecoder => new(); public static readonly string[] CommonTestImages = { @@ -63,16 +65,51 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png TestImages.Png.Bad.ZlibZtxtBadHeader, }; + public static readonly TheoryData PixelFormatRange = new() + { + { TestImages.Png.Gray4Bpp, typeof(Image) }, + { TestImages.Png.L16Bit, typeof(Image) }, + { TestImages.Png.Gray1BitTrans, typeof(Image) }, + { TestImages.Png.Gray2BitTrans, typeof(Image) }, + { TestImages.Png.Gray4BitTrans, typeof(Image) }, + { TestImages.Png.GrayA8Bit, typeof(Image) }, + { TestImages.Png.GrayAlpha16Bit, typeof(Image) }, + { TestImages.Png.Palette8Bpp, typeof(Image) }, + { TestImages.Png.PalettedTwoColor, typeof(Image) }, + { TestImages.Png.Rainbow, typeof(Image) }, + { TestImages.Png.Rgb24BppTrans, typeof(Image) }, + { TestImages.Png.Kaboom, typeof(Image) }, + { TestImages.Png.Rgb48Bpp, typeof(Image) }, + { TestImages.Png.Rgb48BppTrans, typeof(Image) }, + { TestImages.Png.Rgba64Bpp, typeof(Image) }, + }; + + [Theory] + [MemberData(nameof(PixelFormatRange))] + public void Decode_NonGeneric_CreatesCorrectImageType(string path, Type type) + { + string file = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, path); + using var image = Image.Load(file); + Assert.IsType(type, image); + } + + [Theory] + [MemberData(nameof(PixelFormatRange))] + public async Task DecodeAsync_NonGeneric_CreatesCorrectImageType(string path, Type type) + { + string file = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, path); + using Image image = await Image.LoadAsync(file); + Assert.IsType(type, image); + } + [Theory] [WithFileCollection(nameof(CommonTestImages), PixelTypes.Rgba32)] public void Decode(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -81,11 +118,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_GrayWithAlpha(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -95,11 +130,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_Interlaced(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -112,11 +145,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_Indexed(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -125,11 +156,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_48Bpp(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -138,11 +167,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_64Bpp(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -153,11 +180,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decoder_L8bitInterlaced(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -165,11 +190,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_L16Bit(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -178,23 +201,19 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_GrayAlpha16Bit(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] - [WithFile(TestImages.Png.GrayA8BitInterlaced, PixelTypes)] + [WithFile(TestImages.Png.GrayA8BitInterlaced, TestPixelTypes)] public void Decoder_CanDecode_Grey8bitInterlaced_WithAlpha(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -202,23 +221,19 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decoder_CanDecode_CorruptedImages(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] - [WithFile(TestImages.Png.Splash, PixelTypes)] + [WithFile(TestImages.Png.Splash, TestPixelTypes)] public void Decoder_IsNotBoundToSinglePixelType(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); } [Theory] @@ -232,10 +247,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Identify(string imagePath, int expectedPixelSize) { var testFile = TestFile.Create(imagePath); - using (var stream = new MemoryStream(testFile.Bytes, false)) - { - Assert.Equal(expectedPixelSize, Image.Identify(stream)?.PixelType?.BitsPerPixel); - } + using var stream = new MemoryStream(testFile.Bytes, false); + Assert.Equal(expectedPixelSize, Image.Identify(stream)?.PixelType?.BitsPerPixel); } [Theory] @@ -246,10 +259,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png System.Exception ex = Record.Exception( () => { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); }); Assert.NotNull(ex); Assert.Contains("PNG Image does not contain a data chunk", ex.Message); @@ -264,10 +275,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png System.Exception ex = Record.Exception( () => { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); }); Assert.NotNull(ex); Assert.Contains("Invalid or unsupported bit depth", ex.Message); @@ -282,10 +291,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png System.Exception ex = Record.Exception( () => { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); }); Assert.NotNull(ex); Assert.Contains("Invalid or unsupported color type", ex.Message); @@ -300,11 +307,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png System.Exception ex = Record.Exception( () => { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); }); Assert.Null(ex); } @@ -318,11 +323,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png System.Exception ex = Record.Exception( () => { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); }); Assert.Null(ex); } @@ -336,11 +339,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png System.Exception ex = Record.Exception( () => { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); }); Assert.Null(ex); } @@ -354,15 +355,13 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png System.Exception ex = Record.Exception( () => { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); - // We don't have another x-plat reference decoder that can be compared for this image. - if (TestEnvironment.IsWindows) - { - image.CompareToOriginal(provider, ImageComparer.Exact, SystemDrawingReferenceDecoder.Instance); - } + // We don't have another x-plat reference decoder that can be compared for this image. + if (TestEnvironment.IsWindows) + { + image.CompareToOriginal(provider, ImageComparer.Exact, SystemDrawingReferenceDecoder.Instance); } }); Assert.Null(ex); @@ -377,11 +376,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png System.Exception ex = Record.Exception( () => { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); - image.CompareToOriginal(provider, ImageComparer.Exact); - } + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); }); Assert.Null(ex); } @@ -395,15 +392,13 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png System.Exception ex = Record.Exception( () => { - using (Image image = provider.GetImage(PngDecoder)) - { - image.DebugSave(provider); + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); - // We don't have another x-plat reference decoder that can be compared for this image. - if (TestEnvironment.IsWindows) - { - image.CompareToOriginal(provider, ImageComparer.Exact, SystemDrawingReferenceDecoder.Instance); - } + // We don't have another x-plat reference decoder that can be compared for this image. + if (TestEnvironment.IsWindows) + { + image.CompareToOriginal(provider, ImageComparer.Exact, SystemDrawingReferenceDecoder.Instance); } }); Assert.NotNull(ex); From c214af56eb40e3740117dfc429f7b600a0559f9c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 7 Dec 2021 19:35:51 +1100 Subject: [PATCH 06/11] Optimize chunk reading to maximize performance --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 56 +++++++++++++++++--- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index ba737cb42b..71b322df41 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -37,6 +37,11 @@ namespace SixLabors.ImageSharp.Formats.Png /// private readonly bool ignoreMetadata; + /// + /// Gets or sets a value indicating whether to read the header and trns chunks only. + /// + private readonly bool colorMetadataOnly; + /// /// Used the manage memory allocations. /// @@ -124,11 +129,12 @@ namespace SixLabors.ImageSharp.Formats.Png this.ignoreMetadata = options.IgnoreMetadata; } - internal PngDecoderCore(Configuration configuration, bool ignoreMetadata) + internal PngDecoderCore(Configuration configuration, bool colorMetadataOnly) { this.Configuration = configuration ?? Configuration.Default; this.memoryAllocator = this.Configuration.MemoryAllocator; - this.ignoreMetadata = ignoreMetadata; + this.colorMetadataOnly = colorMetadataOnly; + this.ignoreMetadata = true; } /// @@ -137,7 +143,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Gets the dimensions of the image. /// - public Size Dimensions => new Size(this.header.Width, this.header.Height); + public Size Dimensions => new(this.header.Width, this.header.Height); /// public Image Decode(BufferedReadStream stream, CancellationToken cancellationToken) @@ -250,9 +256,21 @@ namespace SixLabors.ImageSharp.Formats.Png this.ReadHeaderChunk(pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.Physical: + if (this.colorMetadataOnly) + { + this.SkipChunkDataAndCrc(chunk); + break; + } + this.ReadPhysicalChunk(metadata, chunk.Data.GetSpan()); break; case PngChunkType.Gamma: + if (this.colorMetadataOnly) + { + this.SkipChunkDataAndCrc(chunk); + break; + } + this.ReadGammaChunk(pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.Data: @@ -265,15 +283,39 @@ namespace SixLabors.ImageSharp.Formats.Png this.AssignTransparentMarkers(alpha, pngMetadata); break; case PngChunkType.Text: + if (this.colorMetadataOnly) + { + this.SkipChunkDataAndCrc(chunk); + break; + } + this.ReadTextChunk(pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.CompressedText: + if (this.colorMetadataOnly) + { + this.SkipChunkDataAndCrc(chunk); + break; + } + this.ReadCompressedTextChunk(pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.InternationalText: + if (this.colorMetadataOnly) + { + this.SkipChunkDataAndCrc(chunk); + break; + } + this.ReadInternationalTextChunk(pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.Exif: + if (this.colorMetadataOnly) + { + this.SkipChunkDataAndCrc(chunk); + break; + } + if (!this.ignoreMetadata) { byte[] exifData = new byte[chunk.Length]; @@ -928,7 +970,7 @@ namespace SixLabors.ImageSharp.Formats.Png int zeroIndex = data.IndexOf((byte)0); // Keywords are restricted to 1 to 79 bytes in length. - if (zeroIndex < PngConstants.MinTextKeywordLength || zeroIndex > PngConstants.MaxTextKeywordLength) + if (zeroIndex is < PngConstants.MinTextKeywordLength or > PngConstants.MaxTextKeywordLength) { return; } @@ -1155,8 +1197,10 @@ namespace SixLabors.ImageSharp.Formats.Png PngChunkType type = this.ReadChunkType(); - // NOTE: Reading the chunk data is the responsible of the caller - if (type == PngChunkType.Data) + // NOTE: Reading the Data chunk is the responsible of the caller + // If we're reading color metadata only we're only interested in the Header and Transparancy chunks. + // We can skip all other chunk data in the stream for better performance. + if (type == PngChunkType.Data || (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency)) { chunk = new PngChunk(length, type); From 7429547d4cdf743c6a270292b085adb113d6d20d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 7 Dec 2021 19:46:04 +1100 Subject: [PATCH 07/11] Faster exit --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 71b322df41..7ffe0cad2e 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -82,6 +82,11 @@ namespace SixLabors.ImageSharp.Formats.Png /// private byte[] paletteAlpha; + /// + /// A value indicating whether the header chunk has been reached. + /// + private bool isHeaderChunkReached; + /// /// A value indicating whether the end chunk has been reached. /// @@ -254,6 +259,7 @@ namespace SixLabors.ImageSharp.Formats.Png { case PngChunkType.Header: this.ReadHeaderChunk(pngMetadata, chunk.Data.GetSpan()); + this.isHeaderChunkReached = true; break; case PngChunkType.Physical: if (this.colorMetadataOnly) @@ -281,6 +287,13 @@ namespace SixLabors.ImageSharp.Formats.Png chunk.Data.GetSpan().CopyTo(alpha); this.paletteAlpha = alpha; this.AssignTransparentMarkers(alpha, pngMetadata); + + if (this.colorMetadataOnly && this.isHeaderChunkReached) + { + // Quick exit + this.isEndChunkReached = true; + } + break; case PngChunkType.Text: if (this.colorMetadataOnly) From ae84ff67e9f6e6f1d2ef5e30645d241098f4a277 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 13 Dec 2021 16:45:40 +0100 Subject: [PATCH 08/11] Break if there are more then 65534 IFD directories, fixes #1897 --- src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs | 12 ++++++++++-- src/ImageSharp/Formats/Tiff/Ifd/EntryReader.cs | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs b/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs index 8b2c6bd3a0..26cb12d425 100644 --- a/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs +++ b/src/ImageSharp/Formats/Tiff/Ifd/DirectoryReader.cs @@ -18,9 +18,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff private uint nextIfdOffset; + private const int DirectoryMax = 65534; + // used for sequential read big values (actual for multiframe big files) // todo: different tags can link to the same data (stream offset) - investigate - private readonly SortedList lazyLoaders = new SortedList(new DuplicateKeyComparer()); + private readonly SortedList lazyLoaders = new(new DuplicateKeyComparer()); public DirectoryReader(Stream stream) => this.stream = stream; @@ -48,7 +50,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff { return ByteOrder.LittleEndian; } - else if (headerBytes[0] == TiffConstants.ByteOrderBigEndian && headerBytes[1] == TiffConstants.ByteOrderBigEndian) + + if (headerBytes[0] == TiffConstants.ByteOrderBigEndian && headerBytes[1] == TiffConstants.ByteOrderBigEndian) { return ByteOrder.BigEndian; } @@ -67,6 +70,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff this.nextIfdOffset = reader.NextIfdOffset; readers.Add(reader); + + if (readers.Count >= DirectoryMax) + { + TiffThrowHelper.ThrowImageFormatException("TIFF image contains too many directories"); + } } // Sequential reading big values. diff --git a/src/ImageSharp/Formats/Tiff/Ifd/EntryReader.cs b/src/ImageSharp/Formats/Tiff/Ifd/EntryReader.cs index 7cd508c09a..be4f59bffc 100644 --- a/src/ImageSharp/Formats/Tiff/Ifd/EntryReader.cs +++ b/src/ImageSharp/Formats/Tiff/Ifd/EntryReader.cs @@ -23,7 +23,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff this.lazyLoaders = lazyLoaders; } - public List Values { get; } = new List(); + public List Values { get; } = new(); public uint NextIfdOffset { get; private set; } From f45e184f87547ad4f031dabffad2c1962f16be07 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 13 Dec 2021 16:52:37 +0100 Subject: [PATCH 09/11] Add test for issue #1891 --- .../Formats/Tiff/TiffDecoderTests.cs | 12 ++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Tiff/Issues/Issue1891.tiff | 3 +++ 3 files changed, 16 insertions(+) create mode 100644 tests/Images/Input/Tiff/Issues/Issue1891.tiff diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index b7bd5275d5..f8256ead9f 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -380,6 +380,18 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff public void TiffDecoder_CanDecode_JpegCompressed(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider, useExactComparer: false); + // https://github.com/SixLabors/ImageSharp/issues/1891 + [Theory] + [WithFile(Issues1891, PixelTypes.Rgba32)] + public void TiffDecoder_ThrowsException_WithTooManyDirectories(TestImageProvider provider) + where TPixel : unmanaged, IPixel => Assert.Throws( + () => + { + using (provider.GetImage(TiffDecoder)) + { + } + }); + [Theory] [WithFileCollection(nameof(MultiframeTestImages), PixelTypes.Rgba32)] public void DecodeMultiframe(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 930b550a28..f43c0c9bd6 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -840,6 +840,7 @@ namespace SixLabors.ImageSharp.Tests public const string Flower32BitGrayPredictorLittleEndian = "Tiff/flower-minisblack-32_lsb_deflate_predictor.tiff"; public const string Issues1716Rgb161616BitLittleEndian = "Tiff/Issues/Issue1716.tiff"; + public const string Issues1891 = "Tiff/Issues/Issue1891.tiff"; public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff"; public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff"; diff --git a/tests/Images/Input/Tiff/Issues/Issue1891.tiff b/tests/Images/Input/Tiff/Issues/Issue1891.tiff new file mode 100644 index 0000000000..df2a5e7987 --- /dev/null +++ b/tests/Images/Input/Tiff/Issues/Issue1891.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:a2779c7fb2c2ad858e0d7efcfffd594cc6fb2846e0475a2998a3cda50f289b9b +size 307 From 07d3653ef508a8819e0520be7aaf63e576c9db91 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 14 Dec 2021 11:41:08 +1100 Subject: [PATCH 10/11] Remove QA from config [skip ci] --- .github/ISSUE_TEMPLATE/config.yml | 3 --- .github/ISSUE_TEMPLATE/oss-bug-report.md | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 1326c72e86..62a8bf2b4f 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,8 +1,5 @@ blank_issues_enabled: false contact_links: - - name: Ask a Question - url: https://github.com/SixLabors/ImageSharp/discussions?discussions_q=category%3AQ%26A - about: Ask a question about this project. - name: Feature Request url: https://github.com/SixLabors/ImageSharp/discussions?discussions_q=category%3AIdeas about: Share ideas for new features for this project. diff --git a/.github/ISSUE_TEMPLATE/oss-bug-report.md b/.github/ISSUE_TEMPLATE/oss-bug-report.md index e0d37de538..9e9567a99a 100644 --- a/.github/ISSUE_TEMPLATE/oss-bug-report.md +++ b/.github/ISSUE_TEMPLATE/oss-bug-report.md @@ -1,6 +1,6 @@ --- name: "OSS : Bug Report" -about: Create a report to help us improve the project. +about: Create a report to help us improve the project. OSS Issues are not guaranteed to be triaged. labels: needs triage --- From f6b83835ad560e5c9ebdb546ff4458c419290cc2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 14 Dec 2021 22:28:54 +1100 Subject: [PATCH 11/11] Update PngDecoderCore.cs --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 53 +++++++++----------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index bc2bb95298..ffaa9d567f 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -38,7 +38,7 @@ namespace SixLabors.ImageSharp.Formats.Png private readonly bool ignoreMetadata; /// - /// Gets or sets a value indicating whether to read the header and trns chunks only. + /// Gets or sets a value indicating whether to read the IHDR and tRNS chunks only. /// private readonly bool colorMetadataOnly; @@ -82,16 +82,6 @@ namespace SixLabors.ImageSharp.Formats.Png /// private byte[] paletteAlpha; - /// - /// A value indicating whether the header chunk has been reached. - /// - private bool isHeaderChunkReached; - - /// - /// A value indicating whether the end chunk has been reached. - /// - private bool isEndChunkReached; - /// /// Previous scanline processed. /// @@ -161,7 +151,7 @@ namespace SixLabors.ImageSharp.Formats.Png Image image = null; try { - while (!this.isEndChunkReached && this.TryReadChunk(out PngChunk chunk)) + while (this.TryReadChunk(out PngChunk chunk)) { try { @@ -215,8 +205,7 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngChunkType.End: - this.isEndChunkReached = true; - break; + goto EOF; case PngChunkType.ProprietaryApple: PngThrowHelper.ThrowInvalidChunkType("Proprietary Apple PNG detected! This PNG file is not conform to the specification and cannot be decoded."); break; @@ -228,6 +217,7 @@ namespace SixLabors.ImageSharp.Formats.Png } } + EOF: if (image is null) { PngThrowHelper.ThrowNoData(); @@ -251,7 +241,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.currentStream.Skip(8); try { - while (!this.isEndChunkReached && this.TryReadChunk(out PngChunk chunk)) + while (this.TryReadChunk(out PngChunk chunk)) { try { @@ -259,7 +249,6 @@ namespace SixLabors.ImageSharp.Formats.Png { case PngChunkType.Header: this.ReadHeaderChunk(pngMetadata, chunk.Data.GetSpan()); - this.isHeaderChunkReached = true; break; case PngChunkType.Physical: if (this.colorMetadataOnly) @@ -280,6 +269,13 @@ namespace SixLabors.ImageSharp.Formats.Png this.ReadGammaChunk(pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.Data: + + // Spec says tRNS must be before IDAT so safe to exit. + if (this.colorMetadataOnly) + { + goto EOF; + } + this.SkipChunkDataAndCrc(chunk); break; case PngChunkType.Transparency: @@ -288,10 +284,9 @@ namespace SixLabors.ImageSharp.Formats.Png this.paletteAlpha = alpha; this.AssignTransparentMarkers(alpha, pngMetadata); - if (this.colorMetadataOnly && this.isHeaderChunkReached) + if (this.colorMetadataOnly) { - // Quick exit - this.isEndChunkReached = true; + goto EOF; } break; @@ -338,8 +333,7 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngChunkType.End: - this.isEndChunkReached = true; - break; + goto EOF; } } finally @@ -347,19 +341,20 @@ namespace SixLabors.ImageSharp.Formats.Png chunk.Data?.Dispose(); // Data is rented in ReadChunkData() } } + + EOF: + if (this.header.Width == 0 && this.header.Height == 0) + { + PngThrowHelper.ThrowNoHeader(); + } + + return new ImageInfo(new PixelTypeInfo(this.CalculateBitsPerPixel()), this.header.Width, this.header.Height, metadata); } finally { this.scanline?.Dispose(); this.previousScanline?.Dispose(); } - - if (this.header.Width == 0 && this.header.Height == 0) - { - PngThrowHelper.ThrowNoHeader(); - } - - return new ImageInfo(new PixelTypeInfo(this.CalculateBitsPerPixel()), this.header.Width, this.header.Height, metadata); } /// @@ -1212,7 +1207,7 @@ namespace SixLabors.ImageSharp.Formats.Png PngChunkType type = this.ReadChunkType(); // NOTE: Reading the Data chunk is the responsible of the caller - // If we're reading color metadata only we're only interested in the Header and Transparancy chunks. + // If we're reading color metadata only we're only interested in the IHDR and tRNS chunks. // We can skip all other chunk data in the stream for better performance. if (type == PngChunkType.Data || (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency)) {