diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs
index 1b21c37f42..9c33d9e784 100644
--- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs
+++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs
@@ -165,6 +165,11 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
this.globalColorTable?.Dispose();
}
+ if (image is null)
+ {
+ GifThrowHelper.ThrowNoData();
+ }
+
return image;
}
@@ -218,6 +223,11 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
this.globalColorTable?.Dispose();
}
+ if (this.logicalScreenDescriptor.Width == 0 && this.logicalScreenDescriptor.Height == 0)
+ {
+ GifThrowHelper.ThrowNoHeader();
+ }
+
return new ImageInfo(
new PixelTypeInfo(this.logicalScreenDescriptor.BitsPerPixel),
this.logicalScreenDescriptor.Width,
@@ -281,40 +291,44 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
// If the length is 11 then it's a valid extension and most likely
// a NETSCAPE, XMP or ANIMEXTS extension. We want the loop count from this.
+ long position = this.stream.Position;
if (appLength == GifConstants.ApplicationBlockSize)
{
this.stream.Read(this.buffer, 0, GifConstants.ApplicationBlockSize);
bool isXmp = this.buffer.AsSpan().StartsWith(GifConstants.XmpApplicationIdentificationBytes);
-
if (isXmp && !this.skipMetadata)
{
- var extension = GifXmpApplicationExtension.Read(this.stream, this.memoryAllocator);
+ GifXmpApplicationExtension extension = GifXmpApplicationExtension.Read(this.stream, this.memoryAllocator);
if (extension.Data.Length > 0)
{
this.metadata.XmpProfile = new XmpProfile(extension.Data);
}
+ else
+ {
+ // Reset the stream position and continue.
+ this.stream.Position = position;
+ this.SkipBlock(appLength);
+ }
return;
}
- else
- {
- int subBlockSize = this.stream.ReadByte();
- // TODO: There's also a NETSCAPE buffer extension.
- // http://www.vurdalakov.net/misc/gif/netscape-buffering-application-extension
- if (subBlockSize == GifConstants.NetscapeLoopingSubBlockSize)
- {
- this.stream.Read(this.buffer, 0, GifConstants.NetscapeLoopingSubBlockSize);
- this.gifMetadata.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.AsSpan(1)).RepeatCount;
- this.stream.Skip(1); // Skip the terminator.
- return;
- }
+ int subBlockSize = this.stream.ReadByte();
- // Could be something else not supported yet.
- // Skip the subblock and terminator.
- this.SkipBlock(subBlockSize);
+ // TODO: There's also a NETSCAPE buffer extension.
+ // http://www.vurdalakov.net/misc/gif/netscape-buffering-application-extension
+ if (subBlockSize == GifConstants.NetscapeLoopingSubBlockSize)
+ {
+ this.stream.Read(this.buffer, 0, GifConstants.NetscapeLoopingSubBlockSize);
+ this.gifMetadata.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.AsSpan(1)).RepeatCount;
+ this.stream.Skip(1); // Skip the terminator.
+ return;
}
+ // Could be something else not supported yet.
+ // Skip the subblock and terminator.
+ this.SkipBlock(subBlockSize);
+
return;
}
@@ -500,7 +514,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
int descriptorBottom = descriptorTop + descriptor.Height;
int descriptorLeft = descriptor.Left;
int descriptorRight = descriptorLeft + descriptor.Width;
- byte transIndex = this.graphicsControlExtension.TransparencyIndex;
+ byte transparentIndex = this.graphicsControlExtension.TransparencyIndex;
int colorTableMaxIdx = colorTable.Length - 1;
for (int y = descriptorTop; y < descriptorBottom && y < imageHeight; y++)
@@ -558,8 +572,10 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
{
for (int x = descriptorLeft; x < descriptorRight && x < imageWidth; x++)
{
- int index = Numerics.Clamp(Unsafe.Add(ref indicesRowRef, x - descriptorLeft), 0, Math.Max(transIndex, colorTableMaxIdx));
- if (transIndex != index)
+ int rawIndex = Unsafe.Add(ref indicesRowRef, x - descriptorLeft);
+ int transIndex = Numerics.Clamp(rawIndex, 0, Math.Max(transparentIndex, colorTableMaxIdx));
+ int index = Numerics.Clamp(rawIndex, 0, colorTableMaxIdx);
+ if (transparentIndex != transIndex)
{
ref TPixel pixel = ref Unsafe.Add(ref rowRef, x);
Rgb24 rgb = colorTable[index];
@@ -649,7 +665,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
this.stream.Skip(6);
this.ReadLogicalScreenDescriptor();
- var meta = new ImageMetadata();
+ ImageMetadata meta = new();
// The Pixel Aspect Ratio is defined to be the quotient of the pixel's
// width over its height. The value range in this field allows
diff --git a/src/ImageSharp/Formats/Gif/GifThrowHelper.cs b/src/ImageSharp/Formats/Gif/GifThrowHelper.cs
index 1a3a86c8dc..66f86a6481 100644
--- a/src/ImageSharp/Formats/Gif/GifThrowHelper.cs
+++ b/src/ImageSharp/Formats/Gif/GifThrowHelper.cs
@@ -1,12 +1,19 @@
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.
+using System.Diagnostics.CodeAnalysis;
+
namespace SixLabors.ImageSharp.Formats.Gif;
internal static class GifThrowHelper
{
+ [DoesNotReturn]
public static void ThrowInvalidImageContentException(string errorMessage)
=> throw new InvalidImageContentException(errorMessage);
- public static void ThrowInvalidImageContentException(string errorMessage, Exception innerException) => throw new InvalidImageContentException(errorMessage, innerException);
+ [DoesNotReturn]
+ public static void ThrowNoHeader() => throw new InvalidImageContentException("Gif image does not contain a Logical Screen Descriptor.");
+
+ [DoesNotReturn]
+ public static void ThrowNoData() => throw new InvalidImageContentException("Unable to read Gif image data");
}
diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs
index 45f3a92805..1c1127c3be 100644
--- a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs
+++ b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs
@@ -28,7 +28,6 @@ internal readonly struct GifXmpApplicationExtension : IGifExtension
/// 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, MemoryAllocator allocator)
{
byte[] xmpBytes = ReadXmpData(stream, allocator);
diff --git a/src/ImageSharp/Formats/ImageDecoderUtilities.cs b/src/ImageSharp/Formats/ImageDecoderUtilities.cs
index c05e0d83cb..a8cbd5aad9 100644
--- a/src/ImageSharp/Formats/ImageDecoderUtilities.cs
+++ b/src/ImageSharp/Formats/ImageDecoderUtilities.cs
@@ -68,6 +68,10 @@ internal static class ImageDecoderUtilities
{
throw new InvalidImageContentException(decoder.Dimensions, ex);
}
+ catch (Exception)
+ {
+ throw;
+ }
}
internal static Image Decode(
@@ -96,6 +100,10 @@ internal static class ImageDecoderUtilities
{
throw largeImageExceptionFactory(ex, decoder.Dimensions);
}
+ catch (Exception)
+ {
+ throw;
+ }
}
private static InvalidImageContentException DefaultLargeImageExceptionFactory(
diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs
index 15fd528e53..7fc61066a7 100644
--- a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs
+++ b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs
@@ -203,8 +203,10 @@ public class GifEncoderTests
}
[Theory]
- [WithFile(TestImages.Gif.Issues.Issue2288OptionalExtension, PixelTypes.Rgba32)]
- [WithFile(TestImages.Gif.Issues.Issue2288OptionalExtension2, PixelTypes.Rgba32)]
+ [WithFile(TestImages.Gif.Issues.Issue2288_A, PixelTypes.Rgba32)]
+ [WithFile(TestImages.Gif.Issues.Issue2288_B, PixelTypes.Rgba32)]
+ [WithFile(TestImages.Gif.Issues.Issue2288_C, PixelTypes.Rgba32)]
+ [WithFile(TestImages.Gif.Issues.Issue2288_D, PixelTypes.Rgba32)]
public void OptionalExtensionsShouldBeHandledProperly(TestImageProvider provider)
where TPixel : unmanaged, IPixel
{
diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs
index 7887c09008..568d5a4054 100644
--- a/tests/ImageSharp.Tests/TestImages.cs
+++ b/tests/ImageSharp.Tests/TestImages.cs
@@ -476,8 +476,10 @@ public static class TestImages
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 const string Issue2288OptionalExtension = "Gif/issues/issue_2288.gif";
- public const string Issue2288OptionalExtension2 = "Gif/issues/issue_2288_2.gif";
+ public const string Issue2288_A = "Gif/issues/issue_2288.gif";
+ public const string Issue2288_B = "Gif/issues/issue_2288_2.gif";
+ public const string Issue2288_C = "Gif/issues/issue_2288_3.gif";
+ public const string Issue2288_D = "Gif/issues/issue_2288_4.gif";
}
public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 };
diff --git a/tests/Images/Input/Gif/issues/issue_2288_3.gif b/tests/Images/Input/Gif/issues/issue_2288_3.gif
new file mode 100644
index 0000000000..fae784409b
--- /dev/null
+++ b/tests/Images/Input/Gif/issues/issue_2288_3.gif
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:d3f0fd68a03e9c1c896e021828f470d9ceeb8f10e1aead230e42e55670520840
+size 958995
diff --git a/tests/Images/Input/Gif/issues/issue_2288_4.gif b/tests/Images/Input/Gif/issues/issue_2288_4.gif
new file mode 100644
index 0000000000..cd975a9202
--- /dev/null
+++ b/tests/Images/Input/Gif/issues/issue_2288_4.gif
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:0ac8bc8677a1eb4e26161e5255a5c651321ff063892f3caec3f95264aee38057
+size 1971226