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 72e3ed144..0a5eba789 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 diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index a21b050a8..e8bb13612 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); } diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 68227db53..1db05bb83 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -68,7 +68,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The pixel array to decode to. public void DecodePixels(int dataSize, Buffer2D pixels) { - Guard.MustBeLessThan(dataSize, int.MaxValue, nameof(dataSize)); + Guard.MustBeLessThanOrEqualTo(1 << dataSize, MaxStackSize, nameof(dataSize)); // The resulting index table length. int width = pixels.Width; diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs index 236508fe9..00bf4b7a2 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 + 269; /// /// Gets the raw Data. @@ -25,40 +28,23 @@ 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); } @@ -67,7 +53,7 @@ namespace SixLabors.ImageSharp.Formats.Gif int totalSize = this.ContentLength; if (buffer.Length < totalSize) { - throw new InsufficientMemoryException("Unable to write XMP metadata to GIF image"); + ThrowInsufficientMemory(); } int bytesWritten = 0; @@ -93,5 +79,29 @@ namespace SixLabors.ImageSharp.Formats.Gif return totalSize; } + + 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); + } + } + + private static void ThrowInsufficientMemory() + => throw new InsufficientMemoryException("Unable to write XMP metadata to GIF image"); } } diff --git a/src/ImageSharp/Metadata/ImageMetadata.cs b/src/ImageSharp/Metadata/ImageMetadata.cs index 89592f776..4760fa141 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(); } /// @@ -175,7 +176,7 @@ namespace SixLabors.ImageSharp.Metadata } /// - public ImageMetadata DeepClone() => new ImageMetadata(this); + public ImageMetadata DeepClone() => new(this); /// /// Synchronizes the profiles with the current metadata. diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index c8ecdb717..7ca64bea7 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -259,5 +259,17 @@ 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); + } } } diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs index 7fba86b4f..eaa7fb564 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; @@ -66,7 +67,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/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 8b943194a..ffdd6f8f2 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -452,6 +452,7 @@ namespace SixLabors.ImageSharp.Tests 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 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/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