From 89ca470b2feca29b0a4c8819748e3a1eb2fa6d4c Mon Sep 17 00:00:00 2001 From: Jason Finch Date: Sun, 18 Oct 2026 09:07:46 +1000 Subject: [PATCH 1/2] Drop GIF frame rows outside the logical screen and stop on LZW codes past the code tables An interlaced frame whose lines fall below the logical screen clamped the target row to the screen height and threw from the row lookup, frames shorter than five lines wrote their lines to the wrong rows, and a frame starting right of the screen threw while slicing the row. Lines below the screen are now decoded and discarded only while a later interlace pass can still reach the screen, and frames starting outside the screen are skipped. With a minimum code size of 12 the first code after a clear code could index past the 4096-entry LZW tables. Decoding and skipping now stop at such a code. --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 30 ++- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 20 ++ .../Formats/Gif/GifDecoderTests.cs | 174 ++++++++++++++++++ 3 files changed, 220 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 0a06f4915b..912a90b60d 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -723,12 +723,14 @@ private void ReadFrameColors( using IMemoryOwner indicesRowOwner = this.memoryAllocator.Allocate(descriptor.Width); Span indicesRow = indicesRowOwner.Memory.Span; + // A frame that starts at or beyond the right or bottom edge of the logical screen has nothing to draw. + // Its image data is then left to the caller, which skips the remaining data sub-blocks. int minCodeSize = stream.ReadByte(); - if (LzwDecoder.IsValidMinCodeSize(minCodeSize)) + if (descriptorTop < imageHeight && descriptorLeft < imageWidth && LzwDecoder.IsValidMinCodeSize(minCodeSize)) { using LzwDecoder lzwDecoder = new(this.configuration.MemoryAllocator, stream, minCodeSize); - for (int y = descriptorTop; y < descriptorBottom && y < imageHeight; y++) + for (int y = descriptorTop; y < descriptorBottom; y++) { // Check if this image is interlaced. int writeY; // the target y offset to write to @@ -737,7 +739,10 @@ private void ReadFrameColors( // If so then we read lines at predetermined offsets. // When an entire image height worth of offset lines has been read we consider this a pass. // With each pass the number of offset lines changes and the starting line changes. - if (interlaceY >= descriptor.Height) + // A frame shorter than 5 lines has passes without any lines, so keep advancing until a + // pass has a line left. The four passes visit every frame line exactly once and the loop + // runs once per frame line, so a pass with a line left always exists. + while (interlaceY >= descriptor.Height) { interlacePass++; switch (interlacePass) @@ -756,7 +761,7 @@ private void ReadFrameColors( } } - writeY = Math.Min(interlaceY + descriptor.Top, image.Height); + writeY = interlaceY + descriptorTop; interlaceY += interlaceIncrement; } else @@ -764,6 +769,23 @@ private void ReadFrameColors( writeY = y; } + // For a properly encoded gif the frame never extends below the logical screen. When it does + // the lines below the screen are dropped. Lines only move down within a pass, so once a line + // falls below the screen the rest of the pass is hidden too. A sequential frame is a single + // pass, so decoding can stop there. Before the last interlace pass, later passes still start + // at frame line 1, 2 or 4 and can reach the screen again unless it shows only frame line 0. + // The hidden lines in between must still be decoded and discarded to reach those lines. + if (writeY >= imageHeight) + { + if (!descriptor.InterlaceFlag || interlacePass == 3 || imageHeight - descriptorTop < 2) + { + break; + } + + lzwDecoder.SkipIndices(descriptor.Width); + continue; + } + lzwDecoder.DecodePixelRow(indicesRow); // #403 The left + width value can be larger than the image width diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index b33d1f3d48..6724e33e86 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -209,6 +209,16 @@ public void DecodePixelRow(Span indices) if (oldCode == NullCode) { + // The only non-literal code that can reach this point is the next available code. With a + // minimum code size of 12 the clear code is 4096, so that code lies past the end of the + // 4096-entry code tables. It has no table entry to output, so the data is corrupt and + // decoding stops. This branch only runs for the first code of the data or after a clear + // code, so the check adds no per-pixel cost. + if (code >= MaxStackSize) + { + break; + } + pixelStack[top++] = suffix[code]; oldCode = code; first = code; @@ -350,6 +360,16 @@ public void SkipIndices(int length) if (oldCode == NullCode) { + // The only non-literal code that can reach this point is the next available code. With a + // minimum code size of 12 the clear code is 4096, so that code lies past the end of the + // 4096-entry code tables. It has no table entry to output, so the data is corrupt and + // decoding stops. This branch only runs for the first code of the data or after a clear + // code, so the check adds no per-pixel cost. + if (code >= MaxStackSize) + { + break; + } + pixelStack[top++] = suffix[code]; oldCode = code; first = code; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 8a8b4866a2..a26fcd2b1d 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -24,6 +24,14 @@ public class GifDecoderTests TestImages.Gif.Giphy, TestImages.Gif.Kumin ]; + /// + /// The global color table of the synthesized frame geometry test images. + /// + private static readonly Rgba32[] GeometryTestColorTable = + [ + new(255, 0, 0), new(0, 255, 0), new(0, 0, 255), new(255, 255, 255) + ]; + [Theory] [WithFileCollection(nameof(MultiFrameTestFiles), PixelTypes.Rgba32)] public void Decode_VerifyAllFrames(TestImageProvider provider) @@ -442,6 +450,172 @@ public void Decode_MalformedApplicationExtension_ThrowsWithStrict() }); } + [Fact] + public void Decode_LzwCodePastCodeTables_StopsDecodingFrame() + { + using Image image = Image.Load(CreateLzwCodePastCodeTablesGif()); + + // No index is decoded so every pixel keeps the cleared index 0. + Assert.Equal(4, image.Width); + Assert.Equal(4, image.Height); + for (int y = 0; y < image.Height; y++) + { + for (int x = 0; x < image.Width; x++) + { + Assert.Equal(GeometryTestColorTable[0], image[x, y]); + } + } + } + + [Fact] + public void Identify_LzwCodePastCodeTables_ReturnsImageInfo() + { + // Identify skips the frame indices so this exercises the index skipping path of the LZW decoder. + ImageInfo info = Image.Identify(CreateLzwCodePastCodeTablesGif()); + + Assert.Equal(4, info.Width); + Assert.Equal(4, info.Height); + } + + [Theory] + [InlineData(20, 20, 0, 0, true)] + [InlineData(20, 20, 0, 0, false)] + [InlineData(10, 10, 0, 5, true)] + [InlineData(10, 10, 0, 5, false)] + [InlineData(10, 10, 5, 3, true)] + [InlineData(10, 10, 0, 10, true)] + [InlineData(10, 10, 0, 10, false)] + [InlineData(10, 20, 0, 8, true)] + [InlineData(10, 20, 0, 9, true)] + [InlineData(10, 1, 0, 0, true)] + [InlineData(10, 2, 0, 0, true)] + [InlineData(10, 3, 0, 7, true)] + [InlineData(10, 4, 0, 0, true)] + [InlineData(4, 4, 12, 0, true)] + [InlineData(4, 4, 12, 0, false)] + public void Decode_FrameExceedingLogicalScreen_WritesOnlyRowsInsideCanvas(int width, int height, int left, int top, bool interlaced) + { + const int screenSize = 10; + + // Each frame row is filled with the color index of its frame row number modulo 4, + // stored in the order an interlaced or sequential frame stores its rows. + int[] rowOrder = interlaced ? GetInterlacedRowOrder(height) : Enumerable.Range(0, height).ToArray(); + using Buffer2D indices = Configuration.Default.MemoryAllocator.Allocate2D(width, height); + for (int i = 0; i < rowOrder.Length; i++) + { + indices.DangerousGetRowSpan(i).Fill((byte)(rowOrder[i] % 4)); + } + + using MemoryStream imageData = new(); + using (LzwEncoder encoder = new(Configuration.Default.MemoryAllocator, 2)) + { + encoder.Encode(indices, imageData); + } + + byte[] data = CreateGeometryTestGif(screenSize, screenSize, left, top, width, height, interlaced, imageData.ToArray()); + + using Image image = Image.Load(data); + + Assert.Equal(screenSize, image.Width); + Assert.Equal(screenSize, image.Height); + for (int y = 0; y < image.Height; y++) + { + for (int x = 0; x < image.Width; x++) + { + // Pixels outside the frame are never written and stay transparent. + bool inFrame = x >= left && x < left + width && y >= top && y < top + height; + Rgba32 expected = inFrame ? GeometryTestColorTable[(y - top) % 4] : default; + Assert.Equal(expected, image[x, y]); + } + } + } + + /// + /// Creates a 4x4 GIF whose frame uses a minimum code size of 12 and holds the single 13-bit code 4098. + /// The clear code is 4096, so the code lies past the 4096-entry LZW code tables. + /// + /// The encoded GIF. + private static byte[] CreateLzwCodePastCodeTablesGif() + => CreateGeometryTestGif(4, 4, 0, 0, 4, 4, false, [12, 0x02, 0x02, 0x10, 0x00]); + + /// + /// Creates a GIF89a with the 4-entry as its global color table + /// and a single image descriptor without a local color table. + /// + /// The logical screen width. + /// The logical screen height. + /// The frame left position. + /// The frame top position. + /// The frame width. + /// The frame height. + /// Whether the frame is interlaced. + /// The image data, from the LZW minimum code size to the block terminator. + /// The encoded GIF. + private static byte[] CreateGeometryTestGif( + int screenWidth, + int screenHeight, + int left, + int top, + int width, + int height, + bool interlaced, + byte[] imageData) + { + using MemoryStream stream = new(); + stream.Write("GIF89a"u8); + + // Logical screen descriptor flagging a 4-entry global color table. + WriteUInt16(stream, screenWidth); + WriteUInt16(stream, screenHeight); + stream.Write([0x81, 0x00, 0x00]); + foreach (Rgba32 color in GeometryTestColorTable) + { + stream.Write([color.R, color.G, color.B]); + } + + // Image descriptor without a local color table. + stream.WriteByte(GifConstants.ImageLabel); + WriteUInt16(stream, left); + WriteUInt16(stream, top); + WriteUInt16(stream, width); + WriteUInt16(stream, height); + stream.WriteByte((byte)(interlaced ? 0x40 : 0x00)); + + stream.Write(imageData); + stream.WriteByte(GifConstants.EndIntroducer); + return stream.ToArray(); + } + + /// + /// Gets the frame rows in the order an interlaced GIF stores them. + /// + /// The frame height. + /// The frame row numbers in storage order. + private static int[] GetInterlacedRowOrder(int height) + { + List rows = []; + foreach ((int start, int step) in new[] { (0, 8), (4, 8), (2, 4), (1, 2) }) + { + for (int y = start; y < height; y += step) + { + rows.Add(y); + } + } + + return [.. rows]; + } + + /// + /// Writes a little-endian 16-bit value. + /// + /// The stream to write to. + /// The value to write. + private static void WriteUInt16(Stream stream, int value) + { + stream.WriteByte((byte)value); + stream.WriteByte((byte)(value >> 8)); + } + private static byte[] CreateGifWithMalformedApplicationExtension() { // The application extension declares a 5-byte application block (0x05), but GIF application From c8d3c4226b5f94f27f4165fa5e732e51df914bea Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 27 Oct 2026 14:05:11 +1000 Subject: [PATCH 2/2] Apply GIF integrity policy and use stored animation regression inputs Reject detected out-of-table LZW codes with InvalidImageContentException. Only IgnoreImageData recovers by retaining completed rows and using the existing sub-block skipper. Keep header and palette parsing outside recovery. Dispose the partially decoded image when decoding fails; the new stored regressions reproduce the missing cleanup on the original main branch. Replace all input generation introduced by this PR with 17 stored GIFs (2310 bytes). Independently verify descriptors, palettes, sub-block boundaries, and malformed code sequences. Pillow checks every geometry fixture's frame samples and following white-pixel frame. Cover code errors before and after clear, hidden-row index skipping, all integrity modes, clipped interlace passes, short interlace passes, and continued animation parsing. Current main fails 13 of the 19 new cases. Release .NET 10: 158 GIF tests and 60 GIF-related/shared decoder tests pass. Final focused run: 19 passed. No reference images or tolerances changed. --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 26 +- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 22 +- .../Formats/Gif/GifDecoderTests.cs | 233 +++++++----------- tests/ImageSharp.Tests/TestImages.cs | 17 ++ .../Gif/frames/10x1-left0-top0-interlaced.gif | 3 + .../frames/10x10-left0-top10-interlaced.gif | 3 + .../frames/10x10-left0-top10-sequential.gif | 3 + .../frames/10x10-left0-top5-interlaced.gif | 3 + .../frames/10x10-left0-top5-sequential.gif | 3 + .../frames/10x10-left5-top3-interlaced.gif | 3 + .../Gif/frames/10x2-left0-top0-interlaced.gif | 3 + .../frames/10x20-left0-top8-interlaced.gif | 3 + .../frames/10x20-left0-top9-interlaced.gif | 3 + .../Gif/frames/10x3-left0-top7-interlaced.gif | 3 + .../Gif/frames/10x4-left0-top0-interlaced.gif | 3 + .../frames/20x20-left0-top0-interlaced.gif | 3 + .../frames/20x20-left0-top0-sequential.gif | 3 + .../Gif/frames/4x4-left12-top0-interlaced.gif | 3 + .../Gif/frames/4x4-left12-top0-sequential.gif | 3 + .../lzw-code-outside-table-hidden-row.gif | 3 + .../Gif/frames/lzw-code-outside-table.gif | 3 + 21 files changed, 196 insertions(+), 153 deletions(-) create mode 100644 tests/Images/Input/Gif/frames/10x1-left0-top0-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/10x10-left0-top10-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/10x10-left0-top10-sequential.gif create mode 100644 tests/Images/Input/Gif/frames/10x10-left0-top5-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/10x10-left0-top5-sequential.gif create mode 100644 tests/Images/Input/Gif/frames/10x10-left5-top3-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/10x2-left0-top0-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/10x20-left0-top8-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/10x20-left0-top9-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/10x3-left0-top7-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/10x4-left0-top0-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/20x20-left0-top0-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/20x20-left0-top0-sequential.gif create mode 100644 tests/Images/Input/Gif/frames/4x4-left12-top0-interlaced.gif create mode 100644 tests/Images/Input/Gif/frames/4x4-left12-top0-sequential.gif create mode 100644 tests/Images/Input/Gif/frames/lzw-code-outside-table-hidden-row.gif create mode 100644 tests/Images/Input/Gif/frames/lzw-code-outside-table.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 912a90b60d..6fc05614c6 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -175,6 +175,12 @@ protected override Image Decode(BufferedReadStream stream, Cance this.gifMetadata.ColorTableMode = FrameColorTableMode.Local; } } + catch + { + // Ownership transfers to the caller only after decoding succeeds. Dispose a partially decoded image on failure. + image?.Dispose(); + throw; + } finally { this.globalColorTable?.Dispose(); @@ -589,7 +595,15 @@ private bool ReadFrame( } } - this.ReadFrameColors(stream, ref image, ref previousFrame, ref previousDisposalMode, colorTable, backgroundColor.ToPixel()); + try + { + this.ReadFrameColors(stream, ref image, ref previousFrame, ref previousDisposalMode, colorTable, backgroundColor.ToPixel()); + } + catch (InvalidImageContentException) when (this.Options.SegmentIntegrityHandling is SegmentIntegrityHandling.IgnoreImageData) + { + // Keep completed rows. SkipBlock below consumes the rest of this frame so the next descriptor remains readable. + // Header and palette parsing stay outside this recovery boundary; structural errors remain fatal. + } // Update from newly decoded frame. FrameDisposalMode disposalMethod = this.graphicsControlExtension.DisposalMethod; @@ -862,7 +876,15 @@ private bool ReadFrameMetadata(BufferedReadStream stream, List /// The pixel indices array to decode to. + /// The first code addresses an entry outside the LZW code tables. public void DecodePixelRow(Span indices) { indices.Clear(); @@ -209,14 +210,12 @@ public void DecodePixelRow(Span indices) if (oldCode == NullCode) { - // The only non-literal code that can reach this point is the next available code. With a - // minimum code size of 12 the clear code is 4096, so that code lies past the end of the - // 4096-entry code tables. It has no table entry to output, so the data is corrupt and - // decoding stops. This branch only runs for the first code of the data or after a clear - // code, so the check adds no per-pixel cost. + // This first code has no previous string to extend. With a tolerated minimum size of 12, + // the next available code is 4098, beyond the 4096-entry tables. Report the corrupt data + // before indexing either table; the GIF decoder applies the image data integrity policy. if (code >= MaxStackSize) { - break; + GifThrowHelper.ThrowInvalidImageContentException("The GIF LZW code exceeds the code table bounds."); } pixelStack[top++] = suffix[code]; @@ -284,6 +283,7 @@ public void DecodePixelRow(Span indices) /// Decodes and decompresses all pixel indices from the stream allowing skipping of the data. /// /// The resulting index table length. + /// The first code addresses an entry outside the LZW code tables. public void SkipIndices(int length) { // Get span values from the owners. @@ -360,14 +360,12 @@ public void SkipIndices(int length) if (oldCode == NullCode) { - // The only non-literal code that can reach this point is the next available code. With a - // minimum code size of 12 the clear code is 4096, so that code lies past the end of the - // 4096-entry code tables. It has no table entry to output, so the data is corrupt and - // decoding stops. This branch only runs for the first code of the data or after a clear - // code, so the check adds no per-pixel cost. + // This first code has no previous string to extend. With a tolerated minimum size of 12, + // the next available code is 4098, beyond the 4096-entry tables. Report the corrupt data + // before indexing either table; the GIF decoder applies the image data integrity policy. if (code >= MaxStackSize) { - break; + GifThrowHelper.ThrowInvalidImageContentException("The GIF LZW code exceeds the code table bounds."); } pixelStack[top++] = suffix[code]; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index a26fcd2b1d..1f826ffe22 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -25,7 +25,7 @@ public class GifDecoderTests ]; /// - /// The global color table of the synthesized frame geometry test images. + /// The four palette colors in the stored frame geometry fixtures. /// private static readonly Rgba32[] GeometryTestColorTable = [ @@ -450,170 +450,125 @@ public void Decode_MalformedApplicationExtension_ThrowsWithStrict() }); } - [Fact] - public void Decode_LzwCodePastCodeTables_StopsDecodingFrame() + /// + /// Invalid LZW table codes fail unless image data errors are explicitly ignored. + /// Recovery retains completed rows and continues to the following animation frame. + /// + /// The stored GIF with an invalid first code or an invalid code after a clear. + /// The number of valid rows before the invalid code. + /// The number of stored animation frames. + [Theory] + [InlineData(TestImages.Gif.LzwCodeOutsideTable, 0, 1)] + [InlineData(TestImages.Gif.LzwCodeOutsideTableHiddenRow, 1, 2)] + public void Decode_LzwCodePastCodeTables_RespectsIntegrityHandling(string path, int completedRows, int frameCount) { - using Image image = Image.Load(CreateLzwCodePastCodeTablesGif()); + byte[] data = TestFile.Create(path).Bytes; - // No index is decoded so every pixel keeps the cleared index 0. - Assert.Equal(4, image.Width); - Assert.Equal(4, image.Height); - for (int y = 0; y < image.Height; y++) + foreach (SegmentIntegrityHandling integrity in Enum.GetValues()) { - for (int x = 0; x < image.Width; x++) + DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; + if (integrity is not SegmentIntegrityHandling.IgnoreImageData) { - Assert.Equal(GeometryTestColorTable[0], image[x, y]); - } - } - } - - [Fact] - public void Identify_LzwCodePastCodeTables_ReturnsImageInfo() - { - // Identify skips the frame indices so this exercises the index skipping path of the LZW decoder. - ImageInfo info = Image.Identify(CreateLzwCodePastCodeTablesGif()); + Assert.Throws(() => + { + using Image image = Image.Load(options, data); + }); - Assert.Equal(4, info.Width); - Assert.Equal(4, info.Height); - } - - [Theory] - [InlineData(20, 20, 0, 0, true)] - [InlineData(20, 20, 0, 0, false)] - [InlineData(10, 10, 0, 5, true)] - [InlineData(10, 10, 0, 5, false)] - [InlineData(10, 10, 5, 3, true)] - [InlineData(10, 10, 0, 10, true)] - [InlineData(10, 10, 0, 10, false)] - [InlineData(10, 20, 0, 8, true)] - [InlineData(10, 20, 0, 9, true)] - [InlineData(10, 1, 0, 0, true)] - [InlineData(10, 2, 0, 0, true)] - [InlineData(10, 3, 0, 7, true)] - [InlineData(10, 4, 0, 0, true)] - [InlineData(4, 4, 12, 0, true)] - [InlineData(4, 4, 12, 0, false)] - public void Decode_FrameExceedingLogicalScreen_WritesOnlyRowsInsideCanvas(int width, int height, int left, int top, bool interlaced) - { - const int screenSize = 10; - - // Each frame row is filled with the color index of its frame row number modulo 4, - // stored in the order an interlaced or sequential frame stores its rows. - int[] rowOrder = interlaced ? GetInterlacedRowOrder(height) : Enumerable.Range(0, height).ToArray(); - using Buffer2D indices = Configuration.Default.MemoryAllocator.Allocate2D(width, height); - for (int i = 0; i < rowOrder.Length; i++) - { - indices.DangerousGetRowSpan(i).Fill((byte)(rowOrder[i] % 4)); - } - - using MemoryStream imageData = new(); - using (LzwEncoder encoder = new(Configuration.Default.MemoryAllocator, 2)) - { - encoder.Encode(indices, imageData); - } - - byte[] data = CreateGeometryTestGif(screenSize, screenSize, left, top, width, height, interlaced, imageData.ToArray()); + continue; + } - using Image image = Image.Load(data); + using Image actual = Image.Load(options, data); + Assert.Equal(new Size(4, frameCount == 1 ? 4 : 2), actual.Size); + Assert.Equal(frameCount, actual.Frames.Count); - Assert.Equal(screenSize, image.Width); - Assert.Equal(screenSize, image.Height); - for (int y = 0; y < image.Height; y++) - { - for (int x = 0; x < image.Width; x++) + for (int y = 0; y < actual.Height; y++) { - // Pixels outside the frame are never written and stay transparent. - bool inFrame = x >= left && x < left + width && y >= top && y < top + height; - Rgba32 expected = inFrame ? GeometryTestColorTable[(y - top) % 4] : default; - Assert.Equal(expected, image[x, y]); + for (int x = 0; x < actual.Width; x++) + { + // A completed row is red. An undecoded row keeps the frame's transparent background. + Assert.Equal(y < completedRows ? new Rgba32(255, 0, 0) : default, actual[x, y]); + + if (frameCount == 2) + { + // The complete green frame after the corrupt frame proves that sub-block recovery resumed correctly. + Assert.Equal(new Rgba32(0, 255, 0), actual.Frames[1][x, y]); + } + } } } } /// - /// Creates a 4x4 GIF whose frame uses a minimum code size of 12 and holds the single 13-bit code 4098. - /// The clear code is 4096, so the code lies past the 4096-entry LZW code tables. + /// Identify uses LZW index skipping and must apply the same image data policy as full decoding. /// - /// The encoded GIF. - private static byte[] CreateLzwCodePastCodeTablesGif() - => CreateGeometryTestGif(4, 4, 0, 0, 4, 4, false, [12, 0x02, 0x02, 0x10, 0x00]); + /// The stored GIF with an invalid table code. + /// The number of stored animation frames. + [Theory] + [InlineData(TestImages.Gif.LzwCodeOutsideTable, 1)] + [InlineData(TestImages.Gif.LzwCodeOutsideTableHiddenRow, 2)] + public void Identify_LzwCodePastCodeTables_RespectsIntegrityHandling(string path, int frameCount) + { + byte[] data = TestFile.Create(path).Bytes; - /// - /// Creates a GIF89a with the 4-entry as its global color table - /// and a single image descriptor without a local color table. - /// - /// The logical screen width. - /// The logical screen height. - /// The frame left position. - /// The frame top position. - /// The frame width. - /// The frame height. - /// Whether the frame is interlaced. - /// The image data, from the LZW minimum code size to the block terminator. - /// The encoded GIF. - private static byte[] CreateGeometryTestGif( - int screenWidth, - int screenHeight, - int left, - int top, - int width, - int height, - bool interlaced, - byte[] imageData) - { - using MemoryStream stream = new(); - stream.Write("GIF89a"u8); - - // Logical screen descriptor flagging a 4-entry global color table. - WriteUInt16(stream, screenWidth); - WriteUInt16(stream, screenHeight); - stream.Write([0x81, 0x00, 0x00]); - foreach (Rgba32 color in GeometryTestColorTable) + foreach (SegmentIntegrityHandling integrity in Enum.GetValues()) { - stream.Write([color.R, color.G, color.B]); - } - - // Image descriptor without a local color table. - stream.WriteByte(GifConstants.ImageLabel); - WriteUInt16(stream, left); - WriteUInt16(stream, top); - WriteUInt16(stream, width); - WriteUInt16(stream, height); - stream.WriteByte((byte)(interlaced ? 0x40 : 0x00)); + DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; + if (integrity is not SegmentIntegrityHandling.IgnoreImageData) + { + Assert.Throws(() => Image.Identify(options, data)); + continue; + } - stream.Write(imageData); - stream.WriteByte(GifConstants.EndIntroducer); - return stream.ToArray(); + ImageInfo info = Image.Identify(options, data); + Assert.Equal(new Size(4, frameCount == 1 ? 4 : 2), info.Size); + Assert.Equal(frameCount, info.FrameMetadataCollection.Count); + } } /// - /// Gets the frame rows in the order an interlaced GIF stores them. + /// Clips stored frame rows to the logical screen without changing interlace order or later frame parsing. /// - /// The frame height. - /// The frame row numbers in storage order. - private static int[] GetInterlacedRowOrder(int height) - { - List rows = []; - foreach ((int start, int step) in new[] { (0, 8), (4, 8), (2, 4), (1, 2) }) + /// The stored GIF with a four-color row pattern and a second frame containing one white pixel. + /// The first frame's stored width. + /// The first frame's stored height. + /// The first frame's left offset. + /// The first frame's top offset. + [Theory] + [InlineData(TestImages.Gif.Frame20x20At0x0Interlaced, 20, 20, 0, 0)] + [InlineData(TestImages.Gif.Frame20x20At0x0Sequential, 20, 20, 0, 0)] + [InlineData(TestImages.Gif.Frame10x10At0x5Interlaced, 10, 10, 0, 5)] + [InlineData(TestImages.Gif.Frame10x10At0x5Sequential, 10, 10, 0, 5)] + [InlineData(TestImages.Gif.Frame10x10At5x3Interlaced, 10, 10, 5, 3)] + [InlineData(TestImages.Gif.Frame10x10At0x10Interlaced, 10, 10, 0, 10)] + [InlineData(TestImages.Gif.Frame10x10At0x10Sequential, 10, 10, 0, 10)] + [InlineData(TestImages.Gif.Frame10x20At0x8Interlaced, 10, 20, 0, 8)] + [InlineData(TestImages.Gif.Frame10x20At0x9Interlaced, 10, 20, 0, 9)] + [InlineData(TestImages.Gif.Frame10x1At0x0Interlaced, 10, 1, 0, 0)] + [InlineData(TestImages.Gif.Frame10x2At0x0Interlaced, 10, 2, 0, 0)] + [InlineData(TestImages.Gif.Frame10x3At0x7Interlaced, 10, 3, 0, 7)] + [InlineData(TestImages.Gif.Frame10x4At0x0Interlaced, 10, 4, 0, 0)] + [InlineData(TestImages.Gif.Frame4x4At12x0Interlaced, 4, 4, 12, 0)] + [InlineData(TestImages.Gif.Frame4x4At12x0Sequential, 4, 4, 12, 0)] + public void Decode_FrameExceedingLogicalScreen_WritesOnlyRowsInsideCanvas(string path, int width, int height, int left, int top) + { + using Image image = Image.Load(TestFile.Create(path).Bytes); + + Assert.Equal(new Size(10, 10), image.Size); + Assert.Equal(2, image.Frames.Count); + + for (int y = 0; y < image.Height; y++) { - for (int y = start; y < height; y += step) + for (int x = 0; x < image.Width; x++) { - rows.Add(y); + // Frame row n contains palette index n modulo four. Pixels outside its rectangle stay transparent. + bool inFrame = x >= left && x < left + width && y >= top && y < top + height; + Rgba32 expected = inFrame ? GeometryTestColorTable[(y - top) % 4] : default; + Assert.Equal(expected, image[x, y]); } } - return [.. rows]; - } - - /// - /// Writes a little-endian 16-bit value. - /// - /// The stream to write to. - /// The value to write. - private static void WriteUInt16(Stream stream, int value) - { - stream.WriteByte((byte)value); - stream.WriteByte((byte)(value >> 8)); + // Even when the first frame is clipped or fully hidden, its data must not consume the next image descriptor. + Assert.Equal(new Rgba32(255, 255, 255), image.Frames[1][0, 0]); } private static byte[] CreateGifWithMalformedApplicationExtension() diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 49b52625c3..b15f7ae0fb 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -588,6 +588,23 @@ public static class Bmp public static class Gif { public const string Rings = "Gif/rings.gif"; + public const string Frame20x20At0x0Interlaced = "Gif/frames/20x20-left0-top0-interlaced.gif"; + public const string Frame20x20At0x0Sequential = "Gif/frames/20x20-left0-top0-sequential.gif"; + public const string Frame10x10At0x5Interlaced = "Gif/frames/10x10-left0-top5-interlaced.gif"; + public const string Frame10x10At0x5Sequential = "Gif/frames/10x10-left0-top5-sequential.gif"; + public const string Frame10x10At5x3Interlaced = "Gif/frames/10x10-left5-top3-interlaced.gif"; + public const string Frame10x10At0x10Interlaced = "Gif/frames/10x10-left0-top10-interlaced.gif"; + public const string Frame10x10At0x10Sequential = "Gif/frames/10x10-left0-top10-sequential.gif"; + public const string Frame10x20At0x8Interlaced = "Gif/frames/10x20-left0-top8-interlaced.gif"; + public const string Frame10x20At0x9Interlaced = "Gif/frames/10x20-left0-top9-interlaced.gif"; + public const string Frame10x1At0x0Interlaced = "Gif/frames/10x1-left0-top0-interlaced.gif"; + public const string Frame10x2At0x0Interlaced = "Gif/frames/10x2-left0-top0-interlaced.gif"; + public const string Frame10x3At0x7Interlaced = "Gif/frames/10x3-left0-top7-interlaced.gif"; + public const string Frame10x4At0x0Interlaced = "Gif/frames/10x4-left0-top0-interlaced.gif"; + public const string Frame4x4At12x0Interlaced = "Gif/frames/4x4-left12-top0-interlaced.gif"; + public const string Frame4x4At12x0Sequential = "Gif/frames/4x4-left12-top0-sequential.gif"; + public const string LzwCodeOutsideTable = "Gif/frames/lzw-code-outside-table.gif"; + public const string LzwCodeOutsideTableHiddenRow = "Gif/frames/lzw-code-outside-table-hidden-row.gif"; public const string Giphy = "Gif/giphy.gif"; public const string Cheers = "Gif/cheers.gif"; public const string Receipt = "Gif/receipt.gif"; diff --git a/tests/Images/Input/Gif/frames/10x1-left0-top0-interlaced.gif b/tests/Images/Input/Gif/frames/10x1-left0-top0-interlaced.gif new file mode 100644 index 0000000000..e352c4728e --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x1-left0-top0-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6f6c613391a57c1a692e112489ba298ddc5b82392ab9c036b513bc5ae30ec49e +size 62 diff --git a/tests/Images/Input/Gif/frames/10x10-left0-top10-interlaced.gif b/tests/Images/Input/Gif/frames/10x10-left0-top10-interlaced.gif new file mode 100644 index 0000000000..e2fad1ab50 --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x10-left0-top10-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:5c3376fe13a98baca43e8646c0bca572910ac9a4b004ca1fd93db9f576962c5c +size 130 diff --git a/tests/Images/Input/Gif/frames/10x10-left0-top10-sequential.gif b/tests/Images/Input/Gif/frames/10x10-left0-top10-sequential.gif new file mode 100644 index 0000000000..49eef35835 --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x10-left0-top10-sequential.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:404e751366a5294da525629c2f7d60c8515967a94e6f09fcbd05ac79a1970041 +size 130 diff --git a/tests/Images/Input/Gif/frames/10x10-left0-top5-interlaced.gif b/tests/Images/Input/Gif/frames/10x10-left0-top5-interlaced.gif new file mode 100644 index 0000000000..4eb97efd87 --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x10-left0-top5-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ff369695cbb6632e5c4e9a80dede1b327847a613c768b5bca40765331fcda874 +size 130 diff --git a/tests/Images/Input/Gif/frames/10x10-left0-top5-sequential.gif b/tests/Images/Input/Gif/frames/10x10-left0-top5-sequential.gif new file mode 100644 index 0000000000..5efc4cf395 --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x10-left0-top5-sequential.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:eb4b005499435b8c4821b9d789ae5eac11269fca7bc03426af0ef2e8e2a80643 +size 130 diff --git a/tests/Images/Input/Gif/frames/10x10-left5-top3-interlaced.gif b/tests/Images/Input/Gif/frames/10x10-left5-top3-interlaced.gif new file mode 100644 index 0000000000..15a8f4f1ae --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x10-left5-top3-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:40a618dc04ac7da59a35201850386c1be6cecf9f9121b3e035fa1db54038a415 +size 130 diff --git a/tests/Images/Input/Gif/frames/10x2-left0-top0-interlaced.gif b/tests/Images/Input/Gif/frames/10x2-left0-top0-interlaced.gif new file mode 100644 index 0000000000..21a43b8c24 --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x2-left0-top0-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ce15534e7cfd3f31b693796e9b45835f048e43a2bba825a0a1964b7fc28f9d78 +size 70 diff --git a/tests/Images/Input/Gif/frames/10x20-left0-top8-interlaced.gif b/tests/Images/Input/Gif/frames/10x20-left0-top8-interlaced.gif new file mode 100644 index 0000000000..28dab8771b --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x20-left0-top8-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:28b10c3d541c9f3d6d572f95b4e119b3e82ca0c5c4bb918811e8202d9ef12006 +size 205 diff --git a/tests/Images/Input/Gif/frames/10x20-left0-top9-interlaced.gif b/tests/Images/Input/Gif/frames/10x20-left0-top9-interlaced.gif new file mode 100644 index 0000000000..4f56d4acf2 --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x20-left0-top9-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:00f9ec0f914dc62f0743216491a47d1d03934beccf6123e78fd8c6238259d499 +size 205 diff --git a/tests/Images/Input/Gif/frames/10x3-left0-top7-interlaced.gif b/tests/Images/Input/Gif/frames/10x3-left0-top7-interlaced.gif new file mode 100644 index 0000000000..3c5d9b66ec --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x3-left0-top7-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:5f1c79f8b08aabe27e67c76e301bab8d318dd2d2229fa99aa2d2a3e645475596 +size 77 diff --git a/tests/Images/Input/Gif/frames/10x4-left0-top0-interlaced.gif b/tests/Images/Input/Gif/frames/10x4-left0-top0-interlaced.gif new file mode 100644 index 0000000000..a378fe268c --- /dev/null +++ b/tests/Images/Input/Gif/frames/10x4-left0-top0-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2d4090fd8a5bcfe4e6ff17c832ccea9ee8e809b31e78df735cd9dc04d64dcfa7 +size 85 diff --git a/tests/Images/Input/Gif/frames/20x20-left0-top0-interlaced.gif b/tests/Images/Input/Gif/frames/20x20-left0-top0-interlaced.gif new file mode 100644 index 0000000000..c24d5dec4b --- /dev/null +++ b/tests/Images/Input/Gif/frames/20x20-left0-top0-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:28d21aa89cfaafc715cfcccba0f22b10dbfc05725d01aecd2d06d50afb15f2f8 +size 356 diff --git a/tests/Images/Input/Gif/frames/20x20-left0-top0-sequential.gif b/tests/Images/Input/Gif/frames/20x20-left0-top0-sequential.gif new file mode 100644 index 0000000000..d24155d986 --- /dev/null +++ b/tests/Images/Input/Gif/frames/20x20-left0-top0-sequential.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:18ce19a92f0e81aaa6e130304709e2a7bc98e95f47b1345867089cc63df1f3ff +size 356 diff --git a/tests/Images/Input/Gif/frames/4x4-left12-top0-interlaced.gif b/tests/Images/Input/Gif/frames/4x4-left12-top0-interlaced.gif new file mode 100644 index 0000000000..8cecbeb6df --- /dev/null +++ b/tests/Images/Input/Gif/frames/4x4-left12-top0-interlaced.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:94172bf7df65de99050f479703d340b28e8b7a92e18d101880150430b3349b1f +size 67 diff --git a/tests/Images/Input/Gif/frames/4x4-left12-top0-sequential.gif b/tests/Images/Input/Gif/frames/4x4-left12-top0-sequential.gif new file mode 100644 index 0000000000..964ee8d0b5 --- /dev/null +++ b/tests/Images/Input/Gif/frames/4x4-left12-top0-sequential.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d22c88886bc90cf83ad13c32047186fe511d9ba7efe1ff16e3a8bc0fc652f766 +size 67 diff --git a/tests/Images/Input/Gif/frames/lzw-code-outside-table-hidden-row.gif b/tests/Images/Input/Gif/frames/lzw-code-outside-table-hidden-row.gif new file mode 100644 index 0000000000..5f97f5d2d6 --- /dev/null +++ b/tests/Images/Input/Gif/frames/lzw-code-outside-table-hidden-row.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:82beb8e9aeadd2cfa04f77cf6d9d872b1004308c33c8d619d284d05fc023af0e +size 69 diff --git a/tests/Images/Input/Gif/frames/lzw-code-outside-table.gif b/tests/Images/Input/Gif/frames/lzw-code-outside-table.gif new file mode 100644 index 0000000000..0c48fe1346 --- /dev/null +++ b/tests/Images/Input/Gif/frames/lzw-code-outside-table.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:78dad5914ed6ec900bddeb839776766accf68a4b64278122434ec171ceb5347d +size 41