From 26ea2af4be2a8bf3083675d417e1028b15db2457 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 6 Oct 2018 13:08:15 +0100 Subject: [PATCH 1/2] Throw when crop rectangle exceeds source bounds. --- ColorSpaceGenerator/ColorSpaceGenerator.csproj | 1 - src/ImageSharp/Processing/CropExtensions.cs | 2 +- .../Processors/Transforms/CropProcessor.cs | 7 +++++-- .../Processors/Transforms/EntropyCropProcessor.cs | 2 +- .../BaseImageOperationsExtensionTest.cs | 8 +++++--- tests/ImageSharp.Tests/ImageOperationTests.cs | 13 +++++-------- .../Processing/Transforms/CropTest.cs | 8 ++++++++ 7 files changed, 25 insertions(+), 16 deletions(-) delete mode 100644 ColorSpaceGenerator/ColorSpaceGenerator.csproj diff --git a/ColorSpaceGenerator/ColorSpaceGenerator.csproj b/ColorSpaceGenerator/ColorSpaceGenerator.csproj deleted file mode 100644 index 7727272f8..000000000 --- a/ColorSpaceGenerator/ColorSpaceGenerator.csproj +++ /dev/null @@ -1 +0,0 @@ - diff --git a/src/ImageSharp/Processing/CropExtensions.cs b/src/ImageSharp/Processing/CropExtensions.cs index 34c754a08..1c0d80afc 100644 --- a/src/ImageSharp/Processing/CropExtensions.cs +++ b/src/ImageSharp/Processing/CropExtensions.cs @@ -35,6 +35,6 @@ namespace SixLabors.ImageSharp.Processing /// The public static IImageProcessingContext Crop(this IImageProcessingContext source, Rectangle cropRectangle) where TPixel : struct, IPixel - => source.ApplyProcessor(new CropProcessor(cropRectangle)); + => source.ApplyProcessor(new CropProcessor(cropRectangle, source.GetCurrentSize())); } } \ No newline at end of file diff --git a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs index 8e6a826fd..3b1d7e94d 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs @@ -22,8 +22,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// Initializes a new instance of the class. /// /// The target cropped rectangle. - public CropProcessor(Rectangle cropRectangle) + /// The source image size. + public CropProcessor(Rectangle cropRectangle, Size sourceSize) { + // Check bounds here and throw if we are passed a rectangle exceeding our source bounds. + Guard.IsTrue(new Rectangle(Point.Empty, sourceSize).Contains(cropRectangle), nameof(cropRectangle), "Crop rectangle should be smaller than the source bounds."); this.CropRectangle = cropRectangle; } @@ -53,7 +56,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms return; } - var rect = Rectangle.Intersect(this.CropRectangle, sourceRectangle); + Rectangle rect = this.CropRectangle; // Copying is cheap, we should process more pixels per task: ParallelExecutionSettings parallelSettings = configuration.GetParallelSettings().MultiplyMinimumPixelsPerTask(4); diff --git a/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor.cs index 8eeae5d1f..6de717afd 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor.cs @@ -62,7 +62,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms rectangle = ImageMaths.GetFilteredBoundingRectangle(temp, 0); } - new CropProcessor(rectangle).Apply(source, sourceRectangle); + new CropProcessor(rectangle, source.Size()).Apply(source, sourceRectangle); } /// diff --git a/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs b/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs index 34b2f718e..7adbefb34 100644 --- a/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs +++ b/tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs @@ -14,7 +14,9 @@ namespace SixLabors.ImageSharp.Tests private readonly FakeImageOperationsProvider.FakeImageOperations internalOperations; protected readonly Rectangle rect; protected readonly GraphicsOptions options; - private Image source; + private readonly Image source; + + public Rectangle SourceBounds() => this.source.Bounds(); public BaseImageOperationsExtensionTest() { @@ -29,7 +31,7 @@ namespace SixLabors.ImageSharp.Tests { Assert.InRange(index, 0, this.internalOperations.Applied.Count - 1); - var operation = this.internalOperations.Applied[index]; + FakeImageOperationsProvider.FakeImageOperations.AppliedOperation operation = this.internalOperations.Applied[index]; return Assert.IsType(operation.Processor); } @@ -38,7 +40,7 @@ namespace SixLabors.ImageSharp.Tests { Assert.InRange(index, 0, this.internalOperations.Applied.Count - 1); - var operation = this.internalOperations.Applied[index]; + FakeImageOperationsProvider.FakeImageOperations.AppliedOperation operation = this.internalOperations.Applied[index]; Assert.Equal(rect, operation.Rectangle); return Assert.IsType(operation.Processor); diff --git a/tests/ImageSharp.Tests/ImageOperationTests.cs b/tests/ImageSharp.Tests/ImageOperationTests.cs index d73eea687..869882f67 100644 --- a/tests/ImageSharp.Tests/ImageOperationTests.cs +++ b/tests/ImageSharp.Tests/ImageOperationTests.cs @@ -56,7 +56,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void CloneCallsImageOperationsProvider_Func_WithDuplicateImage() { - var returned = this.image.Clone(x => x.ApplyProcessor(this.processor)); + Image returned = this.image.Clone(x => x.ApplyProcessor(this.processor)); Assert.True(this.provider.HasCreated(returned)); Assert.Contains(this.processor, this.provider.AppliedOperations(returned).Select(x => x.Processor)); @@ -65,7 +65,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void CloneCallsImageOperationsProvider_ListOfProcessors_WithDuplicateImage() { - var returned = this.image.Clone(this.processor); + Image returned = this.image.Clone(this.processor); Assert.True(this.provider.HasCreated(returned)); Assert.Contains(this.processor, this.provider.AppliedOperations(returned).Select(x => x.Processor)); @@ -74,7 +74,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void CloneCallsImageOperationsProvider_Func_NotOnOrigional() { - var returned = this.image.Clone(x => x.ApplyProcessor(this.processor)); + Image returned = this.image.Clone(x => x.ApplyProcessor(this.processor)); Assert.False(this.provider.HasCreated(this.image)); Assert.DoesNotContain(this.processor, this.provider.AppliedOperations(this.image).Select(x => x.Processor)); } @@ -82,7 +82,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void CloneCallsImageOperationsProvider_ListOfProcessors_NotOnOrigional() { - var returned = this.image.Clone(this.processor); + Image returned = this.image.Clone(this.processor); Assert.False(this.provider.HasCreated(this.image)); Assert.DoesNotContain(this.processor, this.provider.AppliedOperations(this.image).Select(x => x.Processor)); } @@ -95,9 +95,6 @@ namespace SixLabors.ImageSharp.Tests Assert.Contains(this.processor, operations.Applied.Select(x => x.Processor)); } - public void Dispose() - { - this.image.Dispose(); - } + public void Dispose() => this.image.Dispose(); } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Processing/Transforms/CropTest.cs b/tests/ImageSharp.Tests/Processing/Transforms/CropTest.cs index 154167f15..6731debd3 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/CropTest.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/CropTest.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using System; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Transforms; @@ -33,5 +34,12 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms Assert.Equal(cropRectangle, processor.CropRectangle); } + + [Fact] + public void CropRectangleWithInvalidBoundsThrowsException() + { + var cropRectangle = Rectangle.Inflate(this.SourceBounds(), 5, 5); + Assert.Throws(() => this.operations.Crop(cropRectangle)); + } } } \ No newline at end of file From 298a2f038e8d97c231bab325c03f07dfa9d85d40 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 6 Oct 2018 13:28:27 +0100 Subject: [PATCH 2/2] Remove invalid test --- .../Processing/Processors/Transforms/CropTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/CropTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/CropTest.cs index 2f7891512..c01c3b1bd 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/CropTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/CropTest.cs @@ -17,7 +17,6 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms { [Theory] [WithTestPatternImages(70, 30, PixelTypes.Rgba32, 0, 0, 70, 30)] - [WithTestPatternImages(50, 50, PixelTypes.Rgba32, -1, -1, 100, 200)] [WithTestPatternImages(30, 70, PixelTypes.Rgba32, 7, 13, 20, 50)] public void Crop(TestImageProvider provider, int x, int y, int w, int h) where TPixel : struct, IPixel