diff --git a/.github/ISSUE_TEMPLATE/commercial-bug-report.yml b/.github/ISSUE_TEMPLATE/commercial-bug-report.yml index e96677b24..6b4d914d7 100644 --- a/.github/ISSUE_TEMPLATE/commercial-bug-report.yml +++ b/.github/ISSUE_TEMPLATE/commercial-bug-report.yml @@ -4,6 +4,20 @@ description: | Please contact help@sixlabors.com for issues requiring private support. labels: ["commercial", "needs triage"] body: +- type: checkboxes + attributes: + label: Prerequisites + options: + - label: I have bought a Commercial License + required: true + - label: I have written a descriptive issue title + required: true + - label: I have verified that I am running the latest version of ImageSharp + required: true + - label: I have verified if the problem exist in both `DEBUG` and `RELEASE` mode + required: true + - label: I have searched [open](https://github.com/SixLabors/ImageSharp/issues) and [closed](https://github.com/SixLabors/ImageSharp/issues?q=is%3Aissue+is%3Aclosed) issues to ensure it has not already been reported + required: true - type: input attributes: label: ImageSharp version diff --git a/.github/ISSUE_TEMPLATE/oss-bug-report.yml b/.github/ISSUE_TEMPLATE/oss-bug-report.yml index 00138d511..a4e5619d4 100644 --- a/.github/ISSUE_TEMPLATE/oss-bug-report.yml +++ b/.github/ISSUE_TEMPLATE/oss-bug-report.yml @@ -2,6 +2,18 @@ name: "OSS : Bug Report" description: Create a report to help us improve the project. OSS Issues are not guaranteed to be triaged. labels: ["needs triage"] body: +- type: checkboxes + attributes: + label: Prerequisites + options: + - label: I have written a descriptive issue title + required: true + - label: I have verified that I am running the latest version of ImageSharp + required: true + - label: I have verified if the problem exist in both `DEBUG` and `RELEASE` mode + required: true + - label: I have searched [open](https://github.com/SixLabors/ImageSharp/issues) and [closed](https://github.com/SixLabors/ImageSharp/issues?q=is%3Aissue+is%3Aclosed) issues to ensure it has not already been reported + required: true - type: input attributes: label: ImageSharp version diff --git a/ImageSharp.sln b/ImageSharp.sln index 17d293b43..5428f3394 100644 --- a/ImageSharp.sln +++ b/ImageSharp.sln @@ -142,6 +142,13 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Gif", "Gif", "{EE3FB0B3-1C3 EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "issues", "issues", "{BF8DFDC1-CEE5-4A37-B216-D3085360C776}" ProjectSection(SolutionItems) = preProject + tests\Images\Input\Gif\issues\bugzilla-55918.gif = tests\Images\Input\Gif\issues\bugzilla-55918.gif + tests\Images\Input\Gif\issues\issue1505_argumentoutofrange.png = tests\Images\Input\Gif\issues\issue1505_argumentoutofrange.png + tests\Images\Input\Gif\issues\issue1530.gif = tests\Images\Input\Gif\issues\issue1530.gif + tests\Images\Input\Gif\issues\issue1668_invalidcolorindex.gif = tests\Images\Input\Gif\issues\issue1668_invalidcolorindex.gif + tests\Images\Input\Gif\issues\issue1962_tiniest_gif_1st.gif = tests\Images\Input\Gif\issues\issue1962_tiniest_gif_1st.gif + tests\Images\Input\Gif\issues\issue2012_drona1.gif = tests\Images\Input\Gif\issues\issue2012_drona1.gif + tests\Images\Input\Gif\issues\issue2012_Stronghold-Crusader-Extreme-Cover.gif = tests\Images\Input\Gif\issues\issue2012_Stronghold-Crusader-Extreme-Cover.gif tests\Images\Input\Gif\issues\issue403_baddescriptorwidth.gif = tests\Images\Input\Gif\issues\issue403_baddescriptorwidth.gif tests\Images\Input\Gif\issues\issue405_badappextlength252-2.gif = tests\Images\Input\Gif\issues\issue405_badappextlength252-2.gif tests\Images\Input\Gif\issues\issue405_badappextlength252.gif = tests\Images\Input\Gif\issues\issue405_badappextlength252.gif diff --git a/src/ImageSharp/Common/Extensions/StreamExtensions.cs b/src/ImageSharp/Common/Extensions/StreamExtensions.cs index 1193eccee..8746989b3 100644 --- a/src/ImageSharp/Common/Extensions/StreamExtensions.cs +++ b/src/ImageSharp/Common/Extensions/StreamExtensions.cs @@ -13,7 +13,7 @@ namespace SixLabors.ImageSharp internal static class StreamExtensions { /// - /// Writes data from a stream into the provided buffer. + /// Writes data from a stream from the provided buffer. /// /// The stream. /// The buffer. diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index b17ae7f17..dd4f42104 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -265,10 +265,14 @@ namespace SixLabors.ImageSharp.Formats.Gif this.stream.Read(this.buffer, 0, GifConstants.ApplicationBlockSize); bool isXmp = this.buffer.AsSpan().StartsWith(GifConstants.XmpApplicationIdentificationBytes); - if (isXmp) + if (isXmp && !this.IgnoreMetadata) { - var extension = GifXmpApplicationExtension.Read(this.stream); - this.metadata.XmpProfile = new XmpProfile(extension.Data); + var extension = GifXmpApplicationExtension.Read(this.stream, this.MemoryAllocator); + if (extension.Data.Length > 0) + { + this.metadata.XmpProfile = new XmpProfile(extension.Data); + } + return; } else @@ -374,8 +378,8 @@ namespace SixLabors.ImageSharp.Formats.Gif } indices = this.Configuration.MemoryAllocator.Allocate2D(this.imageDescriptor.Width, this.imageDescriptor.Height, AllocationOptions.Clean); - this.ReadFrameIndices(indices); + Span rawColorTable = default; if (localColorTable != null) { @@ -406,9 +410,9 @@ namespace SixLabors.ImageSharp.Formats.Gif [MethodImpl(MethodImplOptions.AggressiveInlining)] private void ReadFrameIndices(Buffer2D indices) { - int dataSize = this.stream.ReadByte(); + int minCodeSize = this.stream.ReadByte(); using var lzwDecoder = new LzwDecoder(this.Configuration.MemoryAllocator, this.stream); - lzwDecoder.DecodePixels(dataSize, indices); + lzwDecoder.DecodePixels(minCodeSize, indices); } /// diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index a21b050a8..da5b1cb23 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -123,7 +123,8 @@ namespace SixLabors.ImageSharp.Formats.Gif this.WriteComments(gifMetadata, stream); // Write application extensions. - this.WriteApplicationExtensions(stream, image.Frames.Count, gifMetadata.RepeatCount, metadata.XmpProfile); + XmpProfile xmpProfile = image.Metadata.XmpProfile ?? image.Frames.RootFrame.Metadata.XmpProfile; + this.WriteApplicationExtensions(stream, image.Frames.Count, gifMetadata.RepeatCount, xmpProfile); if (useGlobalTable) { @@ -137,7 +138,6 @@ namespace SixLabors.ImageSharp.Formats.Gif // Clean up. quantized.Dispose(); - // TODO: Write extension etc stream.WriteByte(GifConstants.EndIntroducer); } @@ -428,26 +428,31 @@ namespace SixLabors.ImageSharp.Formats.Gif where TGifExtension : struct, IGifExtension { IMemoryOwner owner = null; - Span buffer; + Span extensionBuffer; int extensionSize = extension.ContentLength; - if (extensionSize > this.buffer.Length - 3) + + if (extensionSize == 0) + { + return; + } + else if (extensionSize > this.buffer.Length - 3) { owner = this.memoryAllocator.Allocate(extensionSize + 3); - buffer = owner.GetSpan(); + extensionBuffer = owner.GetSpan(); } else { - buffer = this.buffer; + extensionBuffer = this.buffer; } - buffer[0] = GifConstants.ExtensionIntroducer; - buffer[1] = extension.Label; + extensionBuffer[0] = GifConstants.ExtensionIntroducer; + extensionBuffer[1] = extension.Label; - extension.WriteTo(buffer.Slice(2)); + extension.WriteTo(extensionBuffer.Slice(2)); - buffer[extensionSize + 2] = GifConstants.Terminator; + extensionBuffer[extensionSize + 2] = GifConstants.Terminator; - stream.Write(buffer, 0, extensionSize + 3); + stream.Write(extensionBuffer, 0, extensionSize + 3); owner?.Dispose(); } diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 68227db53..2a0720001 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -64,21 +64,30 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// Decodes and decompresses all pixel indices from the stream. /// - /// Size of the data. + /// Minimum code size of the data. /// The pixel array to decode to. - public void DecodePixels(int dataSize, Buffer2D pixels) + public void DecodePixels(int minCodeSize, Buffer2D pixels) { - Guard.MustBeLessThan(dataSize, int.MaxValue, nameof(dataSize)); + // Calculate the clear code. The value of the clear code is 2 ^ minCodeSize + int clearCode = 1 << minCodeSize; + + // It is possible to specify a larger LZW minimum code size than the palette length in bits + // which may leave a gap in the codes where no colors are assigned. + // http://www.matthewflickinger.com/lab/whatsinagif/lzw_image_data.asp#lzw_compression + if (minCodeSize < 2 || clearCode > MaxStackSize) + { + // Don't attempt to decode the frame indices. + // Theoretically we could determine a min code size from the length of the provided + // color palette but we won't bother since the image is most likely corrupted. + GifThrowHelper.ThrowInvalidImageContentException("Gif Image does not contain a valid LZW minimum code."); + } // The resulting index table length. int width = pixels.Width; int height = pixels.Height; int length = width * height; - // Calculate the clear code. The value of the clear code is 2 ^ dataSize - int clearCode = 1 << dataSize; - - int codeSize = dataSize + 1; + int codeSize = minCodeSize + 1; // Calculate the end code int endCode = clearCode + 1; @@ -165,7 +174,7 @@ namespace SixLabors.ImageSharp.Formats.Gif if (code == clearCode) { // Reset the decoder - codeSize = dataSize + 1; + codeSize = minCodeSize + 1; codeMask = (1 << codeSize) - 1; availableCode = clearCode + 2; oldCode = NullCode; diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs index 801849c9b..847633694 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs @@ -71,13 +71,11 @@ namespace SixLabors.ImageSharp.Formats.Gif dest = this; - return 5; + return ((IGifExtension)this).ContentLength; } public static GifGraphicControlExtension Parse(ReadOnlySpan buffer) - { - return MemoryMarshal.Cast(buffer)[0]; - } + => MemoryMarshal.Cast(buffer)[0]; public static byte GetPackedValue(GifDisposalMethod disposalMethod, bool userInputFlag = false, bool transparencyFlag = false) { diff --git a/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs index 2c7bed611..c9e8033db 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs @@ -40,7 +40,7 @@ namespace SixLabors.ImageSharp.Formats.Gif // 0 means loop indefinitely. Count is set as play n + 1 times. BinaryPrimitives.WriteUInt16LittleEndian(buffer.Slice(14, 2), this.RepeatCount); - return 16; // Length - Introducer + Label + Terminator. + return this.ContentLength; // Length - Introducer + Label + Terminator. } } } diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs index 236508fe9..8c396e7fb 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs @@ -2,9 +2,9 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Collections.Generic; using System.IO; -using SixLabors.ImageSharp.Metadata.Profiles.Xmp; +using SixLabors.ImageSharp.IO; +using SixLabors.ImageSharp.Memory; namespace SixLabors.ImageSharp.Formats.Gif { @@ -14,7 +14,10 @@ namespace SixLabors.ImageSharp.Formats.Gif public byte Label => GifConstants.ApplicationExtensionLabel; - public int ContentLength => this.Data.Length + 269; // 12 + Data Length + 1 + 256 + // size : 1 + // identifier : 11 + // magic trailer : 257 + public int ContentLength => (this.Data.Length > 0) ? this.Data.Length + 269 : 0; /// /// Gets the raw Data. @@ -25,51 +28,28 @@ namespace SixLabors.ImageSharp.Formats.Gif /// Reads the XMP metadata from the specified stream. /// /// The stream to read from. + /// The memory allocator. /// The XMP metadata /// Thrown if the XMP block is not properly terminated. - public static GifXmpApplicationExtension Read(Stream stream) + public static GifXmpApplicationExtension Read(Stream stream, MemoryAllocator allocator) { - // Read data in blocks, until an \0 character is encountered. - // We overshoot, indicated by the terminatorIndex variable. - const int bufferSize = 256; - var list = new List(); - int terminationIndex = -1; - while (terminationIndex < 0) - { - byte[] temp = new byte[bufferSize]; - int bytesRead = stream.Read(temp); - list.Add(temp); - terminationIndex = Array.IndexOf(temp, (byte)1); - } + byte[] xmpBytes = ReadXmpData(stream, allocator); - // Pack all the blocks (except magic trailer) into one single array again. - int dataSize = ((list.Count - 1) * bufferSize) + terminationIndex; - byte[] buffer = new byte[dataSize]; - Span bufferSpan = buffer; - int pos = 0; - for (int j = 0; j < list.Count - 1; j++) + // Exclude the "magic trailer", see XMP Specification Part 3, 1.1.2 GIF + int xmpLength = xmpBytes.Length - 256; // 257 - unread 0x0 + byte[] buffer = Array.Empty(); + if (xmpLength > 0) { - list[j].CopyTo(bufferSpan.Slice(pos)); - pos += bufferSize; + buffer = new byte[xmpLength]; + xmpBytes.AsSpan(0, xmpLength).CopyTo(buffer); + stream.Skip(1); // Skip the terminator. } - // Last one only needs the portion until terminationIndex copied over. - Span lastBytes = list[list.Count - 1]; - lastBytes.Slice(0, terminationIndex).CopyTo(bufferSpan.Slice(pos)); - - // Skip the remainder of the magic trailer. - stream.Skip(258 - (bufferSize - terminationIndex)); return new GifXmpApplicationExtension(buffer); } public int WriteTo(Span buffer) { - int totalSize = this.ContentLength; - if (buffer.Length < totalSize) - { - throw new InsufficientMemoryException("Unable to write XMP metadata to GIF image"); - } - int bytesWritten = 0; buffer[bytesWritten++] = GifConstants.ApplicationBlockSize; @@ -91,7 +71,28 @@ namespace SixLabors.ImageSharp.Formats.Gif buffer[bytesWritten++] = 0x00; - return totalSize; + return this.ContentLength; + } + + private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator) + { + using ChunkedMemoryStream bytes = new(allocator); + + // XMP data doesn't have a fixed length nor is there an indicator of the length. + // So we simply read one byte at a time until we hit the 0x0 value at the end + // of the magic trailer or the end of the stream. + // Using ChunkedMemoryStream reduces the array resize allocation normally associated + // with writing from a non fixed-size buffer. + while (true) + { + int b = stream.ReadByte(); + if (b <= 0) + { + return bytes.ToArray(); + } + + bytes.WriteByte((byte)b); + } } } } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index cf2fd0290..e1faf93f4 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -31,7 +31,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// /// Number of component in the current scan. /// - private int componentsCount; + private int scanComponentCount; /// /// The reset interval determined by RST markers. @@ -112,11 +112,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// /// Decodes the entropy coded data. /// - public void ParseEntropyCodedData(int componentCount) + /// Component count in the current scan. + public void ParseEntropyCodedData(int scanComponentCount) { this.cancellationToken.ThrowIfCancellationRequested(); - this.componentsCount = componentCount; + this.scanComponentCount = scanComponentCount; this.scanBuffer = new HuffmanScanBuffer(this.stream); @@ -148,7 +149,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder private void ParseBaselineData() { - if (this.componentsCount != 1) + if (this.scanComponentCount != 1) { this.ParseBaselineDataInterleaved(); this.spectralConverter.CommitConversion(); @@ -180,7 +181,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { // Scan an interleaved mcu... process components in order int mcuCol = mcu % mcusPerLine; - for (int k = 0; k < this.componentsCount; k++) + for (int k = 0; k < this.scanComponentCount; k++) { int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; @@ -228,9 +229,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Convert from spectral to actual pixels via given converter this.spectralConverter.ConvertStrideBaseline(); } - - // Stride conversion must be sealed for stride conversion approach - this.spectralConverter.CommitConversion(); } private void ParseBaselineDataNonInterleaved() @@ -336,7 +334,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } // AC scans may have only one component. - if (this.componentsCount != 1) + if (this.scanComponentCount != 1) { invalid = true; } @@ -368,7 +366,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { this.CheckProgressiveData(); - if (this.componentsCount == 1) + if (this.scanComponentCount == 1) { this.ParseProgressiveDataNonInterleaved(); } @@ -393,7 +391,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Scan an interleaved mcu... process components in order int mcuRow = mcu / mcusPerLine; int mcuCol = mcu % mcusPerLine; - for (int k = 0; k < this.componentsCount; k++) + for (int k = 0; k < this.scanComponentCount; k++) { int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index 40411ef28..430adeb21 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -111,7 +111,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.pixelRowsPerStep = majorVerticalSamplingFactor * blockPixelHeight; // pixel buffer for resulting image - this.pixelBuffer = allocator.Allocate2D(frame.PixelWidth, frame.PixelHeight); + this.pixelBuffer = allocator.Allocate2D( + frame.PixelWidth, + frame.PixelHeight, + this.configuration.PreferContiguousImageBuffers); this.paddedProxyPixelRow = allocator.Allocate(frame.PixelWidth + 3); // component processors from spectral to Rgba32 diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index f5fc86ee4..60437e015 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -429,10 +429,17 @@ namespace SixLabors.ImageSharp.Formats.Png /// The metadata to read to. /// The data containing physical data. private void ReadGammaChunk(PngMetadata pngMetadata, ReadOnlySpan data) + { + if (data.Length < 4) + { + // Ignore invalid gamma chunks. + return; + } - // 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) * 1e-5F; + // The value is encoded as a 4-byte unsigned integer, representing gamma times 100000. + pngMetadata.Gamma = BinaryPrimitives.ReadUInt32BigEndian(data) * 1e-5F; + } /// /// Initializes the image and various buffers needed for processing diff --git a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs index 58fa5aca8..26bc566d6 100644 --- a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs @@ -240,6 +240,11 @@ namespace SixLabors.ImageSharp.Formats.Png byte[] paletteAlpha) where TPixel : unmanaged, IPixel { + if (palette.IsEmpty) + { + PngThrowHelper.ThrowMissingPalette(); + } + TPixel pixel = default; ref byte scanlineSpanRef = ref MemoryMarshal.GetReference(scanlineSpan); ref TPixel rowSpanRef = ref MemoryMarshal.GetReference(rowSpan); diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs index 8700438bd..ae7d16ec7 100644 --- a/src/ImageSharp/Formats/Png/PngThrowHelper.cs +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -21,6 +21,9 @@ namespace SixLabors.ImageSharp.Formats.Png [MethodImpl(InliningOptions.ColdPath)] public static void ThrowNoData() => throw new InvalidImageContentException("PNG Image does not contain a data chunk"); + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain a palette chunk"); + [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidChunkType() => throw new InvalidImageContentException("Invalid PNG data."); diff --git a/src/ImageSharp/Image.Decode.cs b/src/ImageSharp/Image.Decode.cs index ee340bf86..c892b82ba 100644 --- a/src/ImageSharp/Image.Decode.cs +++ b/src/ImageSharp/Image.Decode.cs @@ -37,8 +37,10 @@ namespace SixLabors.ImageSharp ImageMetadata metadata) where TPixel : unmanaged, IPixel { - Buffer2D uninitializedMemoryBuffer = - configuration.MemoryAllocator.Allocate2D(width, height); + Buffer2D uninitializedMemoryBuffer = configuration.MemoryAllocator.Allocate2D( + width, + height, + configuration.PreferContiguousImageBuffers); return new Image(configuration, uninitializedMemoryBuffer.FastMemoryGroup, width, height, metadata); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 16a3cb73d..0da4ff9f8 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -1,11 +1,10 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. using System; using System.Buffers; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Threading; using SixLabors.ImageSharp.Memory.Internals; namespace SixLabors.ImageSharp.Memory @@ -22,7 +21,7 @@ namespace SixLabors.ImageSharp.Memory private readonly int poolCapacity; private readonly UniformUnmanagedMemoryPool.TrimSettings trimSettings; - private UniformUnmanagedMemoryPool pool; + private readonly UniformUnmanagedMemoryPool pool; private readonly UnmanagedMemoryAllocator nonPoolAllocator; public UniformUnmanagedMemoryPoolMemoryAllocator(int? maxPoolSizeMegabytes) @@ -74,6 +73,12 @@ namespace SixLabors.ImageSharp.Memory this.nonPoolAllocator = new UnmanagedMemoryAllocator(unmanagedBufferSizeInBytes); } +#if NETCOREAPP3_1_OR_GREATER + // This delegate allows overriding the method returning the available system memory, + // so we can test our workaround for https://github.com/dotnet/runtime/issues/65466 + internal static Func GetTotalAvailableMemoryBytes { get; set; } = () => GC.GetGCMemoryInfo().TotalAvailableMemoryBytes; +#endif + /// protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; @@ -152,8 +157,13 @@ namespace SixLabors.ImageSharp.Memory // https://github.com/dotnet/runtime/issues/55126#issuecomment-876779327 if (Environment.Is64BitProcess || !RuntimeInformation.FrameworkDescription.StartsWith(".NET 5.0")) { - GCMemoryInfo info = GC.GetGCMemoryInfo(); - return info.TotalAvailableMemoryBytes / 8; + long total = GetTotalAvailableMemoryBytes(); + + // Workaround for https://github.com/dotnet/runtime/issues/65466 + if (total > 0) + { + return total / 8; + } } #endif diff --git a/src/ImageSharp/Metadata/ImageMetadata.cs b/src/ImageSharp/Metadata/ImageMetadata.cs index cddb76d10..d3cb916d4 100644 --- a/src/ImageSharp/Metadata/ImageMetadata.cs +++ b/src/ImageSharp/Metadata/ImageMetadata.cs @@ -68,6 +68,7 @@ namespace SixLabors.ImageSharp.Metadata this.ExifProfile = other.ExifProfile?.DeepClone(); this.IccProfile = other.IccProfile?.DeepClone(); this.IptcProfile = other.IptcProfile?.DeepClone(); + this.XmpProfile = other.XmpProfile?.DeepClone(); } /// diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 250a0e992..e3db8a7ea 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -259,5 +259,46 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); } + + // https://github.com/SixLabors/ImageSharp/issues/2012 + [Theory] + [WithFile(TestImages.Gif.Issues.Issue2012EmptyXmp, PixelTypes.Rgba32)] + public void Issue2012EmptyXmp(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + + image.DebugSave(provider); + image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); + } + + // https://github.com/SixLabors/ImageSharp/issues/2012 + [Theory] + [WithFile(TestImages.Gif.Issues.Issue2012BadMinCode, PixelTypes.Rgba32)] + public void Issue2012BadMinCode(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(); + image.DebugSave(provider); + }); + + Assert.NotNull(ex); + Assert.Contains("Gif Image does not contain a valid LZW minimum code.", ex.Message); + } + + // https://bugzilla.mozilla.org/show_bug.cgi?id=55918 + [Theory] + [WithFile(TestImages.Gif.Issues.DeferredClearCode, PixelTypes.Rgba32)] + public void IssueDeferredClearCode(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + + image.DebugSave(provider); + image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); + } } } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index c29f8c589..0af5d9995 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -265,6 +265,36 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png Assert.Contains("PNG Image does not contain a data chunk", ex.Message); } + [Theory] + [WithFile(TestImages.Png.Bad.MissingPaletteChunk1, PixelTypes.Rgba32)] + [WithFile(TestImages.Png.Bad.MissingPaletteChunk2, PixelTypes.Rgba32)] + public void Decode_MissingPaletteChunk_ThrowsException(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + }); + Assert.NotNull(ex); + Assert.Contains("PNG Image does not contain a palette chunk", ex.Message); + } + + [Theory] + [WithFile(TestImages.Png.Bad.InvalidGammaChunk, PixelTypes.Rgba32)] + public void Decode_InvalidGammaChunk_Ignored(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + }); + Assert.Null(ex); + } + [Theory] [WithFile(TestImages.Png.Bad.BitDepthZero, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Bad.BitDepthThree, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs index 99bc15228..0064e6da7 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System.IO; +using System.Threading.Tasks; using SixLabors.ImageSharp.Formats.Webp; using SixLabors.ImageSharp.Metadata.Profiles.Exif; using SixLabors.ImageSharp.PixelFormats; @@ -86,7 +87,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Webp [Theory] [WithFile(TestImages.Webp.Lossy.WithXmp, PixelTypes.Rgba32, false)] [WithFile(TestImages.Webp.Lossy.WithXmp, PixelTypes.Rgba32, true)] - public async void IgnoreMetadata_ControlsWhetherXmpIsParsed(TestImageProvider provider, bool ignoreMetadata) + public async Task IgnoreMetadata_ControlsWhetherXmpIsParsed(TestImageProvider provider, bool ignoreMetadata) where TPixel : unmanaged, IPixel { var decoder = new WebpDecoder { IgnoreMetadata = ignoreMetadata }; diff --git a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs index b2ee9d673..357d02be4 100644 --- a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs +++ b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs @@ -2,7 +2,10 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.IO; using Microsoft.DotNet.RemoteExecutor; +using SixLabors.ImageSharp.Advanced; +using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; @@ -32,9 +35,40 @@ namespace SixLabors.ImageSharp.Tests Configuration configuration = Configuration.Default.Clone(); configuration.PreferContiguousImageBuffers = true; - using var image = new Image(configuration, 8192, 4096); + using var image = new Image(configuration, 2048, 2048); Assert.True(image.DangerousTryGetSinglePixelMemory(out Memory mem)); - Assert.Equal(8192 * 4096, mem.Length); + Assert.Equal(2048 * 2048, mem.Length); + } + } + + [Theory] + [InlineData("bmp")] + [InlineData("png")] + [InlineData("jpeg")] + [InlineData("gif")] + [InlineData("tiff")] + [InlineData("webp")] + public void PreferContiguousImageBuffers_LoadImage_BufferIsContiguous(string formatOuter) + { + // Run remotely to avoid large allocation in the test process: + RemoteExecutor.Invoke(RunTest, formatOuter).Dispose(); + + static void RunTest(string formatInner) + { + Configuration configuration = Configuration.Default.Clone(); + configuration.PreferContiguousImageBuffers = true; + IImageEncoder encoder = configuration.ImageFormatsManager.FindEncoder( + configuration.ImageFormatsManager.FindFormatByFileExtension(formatInner)); + string dir = TestEnvironment.CreateOutputDirectory(".Temp"); + string path = Path.Combine(dir, $"{Guid.NewGuid().ToString()}.{formatInner}"); + using (Image temp = new(2048, 2048)) + { + temp.Save(path, encoder); + } + + using var image = Image.Load(configuration, path); + File.Delete(path); + Assert.Equal(1, image.GetPixelMemoryGroup().Count); } } diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 45a7cc278..414f991f7 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -379,5 +379,20 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators g1.GetSpan()[0] = 42; } } + +#if NETCOREAPP3_1_OR_GREATER + [Fact] + public void Issue2001_NegativeMemoryReportedByGc() + { + RemoteExecutor.Invoke(RunTest).Dispose(); + + static void RunTest() + { + // Emulate GC.GetGCMemoryInfo() issue https://github.com/dotnet/runtime/issues/65466 + UniformUnmanagedMemoryPoolMemoryAllocator.GetTotalAvailableMemoryBytes = () => -402354176; + _ = MemoryAllocator.Create(); + } + } +#endif } } diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs index 81dad699a..a3c15d0b3 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs +++ b/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs @@ -4,6 +4,7 @@ using System; using System.IO; using System.Text; +using System.Threading.Tasks; using System.Xml.Linq; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Gif; @@ -31,7 +32,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [Theory] [WithFile(TestImages.Gif.Receipt, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromGif_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromGif_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(GifDecoder)) @@ -45,7 +46,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [WithFile(TestImages.Jpeg.Baseline.Lake, PixelTypes.Rgba32)] [WithFile(TestImages.Jpeg.Baseline.Metadata, PixelTypes.Rgba32)] [WithFile(TestImages.Jpeg.Baseline.ExtendedXmp, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromJpg_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromJpg_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(JpegDecoder)) @@ -57,7 +58,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [Theory] [WithFile(TestImages.Png.XmpColorPalette, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromPng_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromPng_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(PngDecoder)) @@ -69,7 +70,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [Theory] [WithFile(TestImages.Tiff.SampleMetadata, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromTiff_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromTiff_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(TiffDecoder)) @@ -81,7 +82,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [Theory] [WithFile(TestImages.Webp.Lossy.WithXmp, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromWebp_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromWebp_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(WebpDecoder)) @@ -157,7 +158,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp } [Fact] - public async void WritingJpeg_PreservesExtendedXmpProfile() + public async Task WritingJpeg_PreservesExtendedXmpProfile() { // arrange var provider = TestImageProvider.File(TestImages.Jpeg.Baseline.ExtendedXmp); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index eeaae253a..f16407ee8 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -127,6 +127,9 @@ namespace SixLabors.ImageSharp.Tests public const string MissingDataChunk = "Png/xdtn0g01.png"; public const string WrongCrcDataChunk = "Png/xcsn0g01.png"; public const string CorruptedChunk = "Png/big-corrupted-chunk.png"; + public const string MissingPaletteChunk1 = "Png/missing_plte.png"; + public const string MissingPaletteChunk2 = "Png/missing_plte_2.png"; + public const string InvalidGammaChunk = "Png/length_gama.png"; // Zlib errors. public const string ZlibOverflow = "Png/zlib-overflow.png"; @@ -448,10 +451,13 @@ namespace SixLabors.ImageSharp.Tests public const string BadAppExtLength = "Gif/issues/issue405_badappextlength252.gif"; public const string BadAppExtLength_2 = "Gif/issues/issue405_badappextlength252-2.gif"; public const string BadDescriptorWidth = "Gif/issues/issue403_baddescriptorwidth.gif"; + public const string DeferredClearCode = "Gif/issues/bugzilla-55918.gif"; public const string Issue1505 = "Gif/issues/issue1505_argumentoutofrange.png"; public const string Issue1530 = "Gif/issues/issue1530.gif"; public const string InvalidColorIndex = "Gif/issues/issue1668_invalidcolorindex.gif"; public const string Issue1962NoColorTable = "Gif/issues/issue1962_tiniest_gif_1st.gif"; + public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif"; + public const string Issue2012BadMinCode = "Gif/issues/issue2012_drona1.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png new file mode 100644 index 000000000..c646eb860 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:3ba8295d8a4b087d6c19fbad7e97cef7b5ce1a69b9c4c4f79cee6bc77e41f236 +size 62778 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png new file mode 100644 index 000000000..b5769c2c4 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6b33733518b855b25c5e9a1b2f5c93cacf0699a40a459dde795b0ed91a978909 +size 12776 diff --git a/tests/Images/Input/Gif/issues/bugzilla-55918.gif b/tests/Images/Input/Gif/issues/bugzilla-55918.gif new file mode 100644 index 000000000..929ea67c3 --- /dev/null +++ b/tests/Images/Input/Gif/issues/bugzilla-55918.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d11148669a093c2e39be62bc3482c5863362d28c03c7f26c5a2386d5de28c339 +size 14551 diff --git a/tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif b/tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif new file mode 100644 index 000000000..90f0e0f1c --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:97f8fdbabfbd9663bf9940dc33f81edf330b62789d1aa573ae85a520903723e5 +size 77498 diff --git a/tests/Images/Input/Gif/issues/issue2012_drona1.gif b/tests/Images/Input/Gif/issues/issue2012_drona1.gif new file mode 100644 index 000000000..803d68487 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue2012_drona1.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cbb23b2a19e314969c6da99374ae133d834d76c3f0ab9df4a7edc9334bb065e6 +size 10508 diff --git a/tests/Images/Input/Png/length_gama.png b/tests/Images/Input/Png/length_gama.png new file mode 100644 index 000000000..caf0fb01d --- /dev/null +++ b/tests/Images/Input/Png/length_gama.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:824766b34739727c722e88611d7b55401452c2970cd433f56e5f9f1b36d6950d +size 1285 diff --git a/tests/Images/Input/Png/missing_plte.png b/tests/Images/Input/Png/missing_plte.png new file mode 100644 index 000000000..0c24883fb --- /dev/null +++ b/tests/Images/Input/Png/missing_plte.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:73fd17a394f8258f4767986bc427c0160277819349c937f18cb29044e7549bc8 +size 506 diff --git a/tests/Images/Input/Png/missing_plte_2.png b/tests/Images/Input/Png/missing_plte_2.png new file mode 100644 index 000000000..8fc6580e5 --- /dev/null +++ b/tests/Images/Input/Png/missing_plte_2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:797844db61a937c6f31ecb392c8416fbf106017413ba55c6576e0b1fcfc1cf9c +size 597