From 228d02f39c920bb64332c5828fb389f9671d4d85 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 28 Apr 2020 10:15:34 +0200 Subject: [PATCH 1/3] Throw exception when width or height is zero --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index de5aa78843..e0d8b3cd96 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -6,7 +6,7 @@ using System.IO; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; -using SixLabors.ImageSharp.Advanced; + using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; @@ -101,7 +101,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// Decodes the stream to the image. /// /// The pixel format. - /// The stream containing image data. + /// The stream containing image data. /// The decoded image public Image Decode(Stream stream) where TPixel : unmanaged, IPixel @@ -241,6 +241,10 @@ namespace SixLabors.ImageSharp.Formats.Gif this.stream.Read(this.buffer, 0, 9); this.imageDescriptor = GifImageDescriptor.Parse(this.buffer); + if (this.imageDescriptor.Height == 0 || this.imageDescriptor.Width == 0) + { + GifThrowHelper.ThrowInvalidImageContentException("Width or height should not be 0"); + } } /// From fcfd662e81ef054c5ac36788662470cf9d14589f Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 28 Apr 2020 10:15:58 +0200 Subject: [PATCH 2/3] Additional gif test cases --- src/ImageSharp/Formats/Gif/README.md | 8 +- .../Formats/Gif/GifDecoderTests.cs | 82 +++++++++++-------- tests/ImageSharp.Tests/TestImages.cs | 7 ++ tests/Images/Input/Gif/image-zero-height.gif | 3 + tests/Images/Input/Gif/image-zero-size.gif | 3 + tests/Images/Input/Gif/image-zero-width.gif | 3 + tests/Images/Input/Gif/max-height.gif | 3 + tests/Images/Input/Gif/max-width.gif | 3 + 8 files changed, 74 insertions(+), 38 deletions(-) create mode 100644 tests/Images/Input/Gif/image-zero-height.gif create mode 100644 tests/Images/Input/Gif/image-zero-size.gif create mode 100644 tests/Images/Input/Gif/image-zero-width.gif create mode 100644 tests/Images/Input/Gif/max-height.gif create mode 100644 tests/Images/Input/Gif/max-width.gif diff --git a/src/ImageSharp/Formats/Gif/README.md b/src/ImageSharp/Formats/Gif/README.md index d47a4c6836..eeda20c06a 100644 --- a/src/ImageSharp/Formats/Gif/README.md +++ b/src/ImageSharp/Formats/Gif/README.md @@ -1,4 +1,6 @@ -Encoder/Decoder adapted and extended from: +Encoder/Decoder adapted and extended from: -https://github.com/yufeih/Nine.Imaging/ -https://imagetools.codeplex.com/ +- [Nine.Imaging](https://github.com/yufeih/Nine.Imaging/) +- [imagetools.codeplex](https://imagetools.codeplex.com/) + +A useful set of gif test images can be found at [pygif](https://github.com/robert-ancell/pygif/tree/master/test-suite) \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index b3a99aa1c0..199c6c0b25 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -2,11 +2,9 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Collections.Generic; using System.IO; using Microsoft.DotNet.RemoteExecutor; -using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Formats.Gif; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; @@ -30,32 +28,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif TestImages.Gif.Giphy, TestImages.Gif.Kumin }; - public static readonly string[] BasicVerificationFiles = - { - TestImages.Gif.Cheers, - TestImages.Gif.Rings, - - // previously DecodeBadApplicationExtensionLength: - TestImages.Gif.Issues.BadAppExtLength, - TestImages.Gif.Issues.BadAppExtLength_2, - - // previously DecodeBadDescriptorDimensionsLength: - TestImages.Gif.Issues.BadDescriptorWidth - }; - - private static readonly Dictionary BasicVerificationFrameCount = - new Dictionary - { - [TestImages.Gif.Cheers] = 93, - [TestImages.Gif.Issues.BadDescriptorWidth] = 36, - }; - - public static readonly string[] BadAppExtFiles = - { - TestImages.Gif.Issues.BadAppExtLength, - TestImages.Gif.Issues.BadAppExtLength_2 - }; - [Theory] [WithFileCollection(nameof(MultiFrameTestFiles), PixelTypes.Rgba32)] public void Decode_VerifyAllFrames(TestImageProvider provider) @@ -100,15 +72,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif } [Theory] - [WithFileCollection(nameof(BasicVerificationFiles), PixelTypes.Rgba32)] - public void Decode_VerifyRootFrameAndFrameCount(TestImageProvider provider) + [WithFile(TestImages.Gif.Cheers, PixelTypes.Rgba32, 93)] + [WithFile(TestImages.Gif.Rings, PixelTypes.Rgba32, 1)] + [WithFile(TestImages.Gif.Issues.BadDescriptorWidth, PixelTypes.Rgba32, 36)] + public void Decode_VerifyRootFrameAndFrameCount(TestImageProvider provider, int expectedFrameCount) where TPixel : unmanaged, IPixel { - if (!BasicVerificationFrameCount.TryGetValue(provider.SourceFileOrDescription, out int expectedFrameCount)) - { - expectedFrameCount = 1; - } - using (Image image = provider.GetImage()) { Assert.Equal(expectedFrameCount, image.Frames.Count); @@ -153,6 +122,35 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif } } + [Theory] + [WithFile(TestImages.Gif.ZeroSize, PixelTypes.Rgba32)] + [WithFile(TestImages.Gif.ZeroWidth, PixelTypes.Rgba32)] + [WithFile(TestImages.Gif.ZeroHeight, PixelTypes.Rgba32)] + public void Decode_WithInvalidDimensions_DoesThrowException(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + System.Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(GifDecoder); + }); + Assert.NotNull(ex); + Assert.Contains("Width or height should not be 0", ex.Message); + } + + [Theory] + [WithFile(TestImages.Gif.MaxWidth, PixelTypes.Rgba32, 65535, 1)] + [WithFile(TestImages.Gif.MaxHeight, PixelTypes.Rgba32, 1, 65535)] + public void Decode_WithMaxDimensions_Works(TestImageProvider provider, int expectedWidth, int expectedHeight) + where TPixel : unmanaged, IPixel + { + using (Image image = provider.GetImage(GifDecoder)) + { + Assert.Equal(expectedWidth, image.Width); + Assert.Equal(expectedHeight, image.Height); + } + } + [Fact] public void CanDecodeIntermingledImages() { @@ -172,6 +170,20 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif } } + // https://github.com/SixLabors/ImageSharp/issues/405 + [Theory] + [WithFile(TestImages.Gif.Issues.BadAppExtLength, PixelTypes.Rgba32)] + [WithFile(TestImages.Gif.Issues.BadAppExtLength_2, PixelTypes.Rgba32)] + public void Issue405_BadApplicationExtensionBlockLength(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using (Image image = provider.GetImage()) + { + image.DebugSave(provider); + image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); + } + } + [Theory] [WithFile(TestImages.Gif.Giphy, PixelTypes.Rgba32)] [WithFile(TestImages.Gif.Kumin, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index bec0c66242..141a8d1c38 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -397,6 +397,13 @@ namespace SixLabors.ImageSharp.Tests public const string Ratio1x4 = "Gif/base_1x4.gif"; public const string LargeComment = "Gif/large_comment.gif"; + // Test images from https://github.com/robert-ancell/pygif/tree/master/test-suite + public const string ZeroSize = "Gif/image-zero-size.gif"; + public const string ZeroHeight = "Gif/image-zero-height.gif"; + public const string ZeroWidth = "Gif/image-zero-width.gif"; + public const string MaxWidth = "Gif/max-width.gif"; + public const string MaxHeight = "Gif/max-height.gif"; + public static class Issues { public const string BadAppExtLength = "Gif/issues/issue405_badappextlength252.gif"; diff --git a/tests/Images/Input/Gif/image-zero-height.gif b/tests/Images/Input/Gif/image-zero-height.gif new file mode 100644 index 0000000000..f4f70ab6a4 --- /dev/null +++ b/tests/Images/Input/Gif/image-zero-height.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:37248eeb127e43bb002f621409cb6dabaa6b58a62612d26009722c4ae7c83dd6 +size 30 diff --git a/tests/Images/Input/Gif/image-zero-size.gif b/tests/Images/Input/Gif/image-zero-size.gif new file mode 100644 index 0000000000..c2bccffc49 --- /dev/null +++ b/tests/Images/Input/Gif/image-zero-size.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:518aa6f50b003b76e8b65e798d2a37b6dad7dade96d0a7db73da88eec07efe0e +size 30 diff --git a/tests/Images/Input/Gif/image-zero-width.gif b/tests/Images/Input/Gif/image-zero-width.gif new file mode 100644 index 0000000000..642be49ad4 --- /dev/null +++ b/tests/Images/Input/Gif/image-zero-width.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4cde31fe4bcc863f70f66c5a57d62647b11512920328fc5658399ef566ebebef +size 30 diff --git a/tests/Images/Input/Gif/max-height.gif b/tests/Images/Input/Gif/max-height.gif new file mode 100644 index 0000000000..fcec4bd936 --- /dev/null +++ b/tests/Images/Input/Gif/max-height.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:8853634077f425f4b2077f4ca0c986e4ac0e549a8601859b0578a3ccbdbdd5d4 +size 405 diff --git a/tests/Images/Input/Gif/max-width.gif b/tests/Images/Input/Gif/max-width.gif new file mode 100644 index 0000000000..bb0e131acc --- /dev/null +++ b/tests/Images/Input/Gif/max-width.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:008e2a84afed6c31b6635aa9d8c7ee2176f01a0eb0a04143883a8533d7ca33c9 +size 405 From 1b9dc4a885f8da422fcd9cff89d5d9ab35487cfd Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 28 Apr 2020 17:33:16 +0200 Subject: [PATCH 3/3] Update external due to test file name changes --- tests/Images/External | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Images/External b/tests/Images/External index 6fdc6d19b1..0d1f91e2fe 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit 6fdc6d19b101dc1c00a297d3e92257df60c413d0 +Subproject commit 0d1f91e2fe1491f6dc2c137a8ea20460fde4404c