Merge pull request #726 from SixLabors/js/fix-605

Throw when crop rectangle exceeds source bounds.

ColorSpaceGenerator/ColorSpaceGenerator.csproj

@@ -1 +0,0 @@
-  <ItemGroup>

src/ImageSharp/Processing/CropExtensions.cs

@@ -35,6 +35,6 @@ namespace SixLabors.ImageSharp.Processing
         /// <returns>The <see cref="Image{TPixel}"/></returns>
         public static IImageProcessingContext<TPixel> Crop<TPixel>(this IImageProcessingContext<TPixel> source, Rectangle cropRectangle)
             where TPixel : struct, IPixel<TPixel>
-            => source.ApplyProcessor(new CropProcessor<TPixel>(cropRectangle));
+            => source.ApplyProcessor(new CropProcessor<TPixel>(cropRectangle, source.GetCurrentSize()));
     }
 }

src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs

@@ -22,8 +22,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms
         /// Initializes a new instance of the <see cref="CropProcessor{TPixel}"/> class.
         /// </summary>
         /// <param name="cropRectangle">The target cropped rectangle.</param>
-        public CropProcessor(Rectangle cropRectangle)
+        /// <param name="sourceSize">The source image size.</param>
+        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);

src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor.cs

@@ -62,7 +62,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms
                 rectangle = ImageMaths.GetFilteredBoundingRectangle(temp, 0);
             }
 
-            new CropProcessor<TPixel>(rectangle).Apply(source, sourceRectangle);
+            new CropProcessor<TPixel>(rectangle, source.Size()).Apply(source, sourceRectangle);
         }
 
         /// <inheritdoc/>

tests/ImageSharp.Tests/BaseImageOperationsExtensionTest.cs

@@ -14,7 +14,9 @@ namespace SixLabors.ImageSharp.Tests
         private readonly FakeImageOperationsProvider.FakeImageOperations<Rgba32> internalOperations;
         protected readonly Rectangle rect;
         protected readonly GraphicsOptions options;
-        private Image<Rgba32> source;
+        private readonly Image<Rgba32> 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<Rgba32>.AppliedOperation operation = this.internalOperations.Applied[index];
 
             return Assert.IsType<T>(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<Rgba32>.AppliedOperation operation = this.internalOperations.Applied[index];
 
             Assert.Equal(rect, operation.Rectangle);
             return Assert.IsType<T>(operation.Processor);

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<Rgba32> 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<Rgba32> 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<Rgba32> 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<Rgba32> 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();
     }
 }

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<TPixel>(TestImageProvider<TPixel> provider, int x, int y, int w, int h)
             where TPixel : struct, IPixel<TPixel>

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<ArgumentException>(() => this.operations.Crop(cropRectangle));
+        }
     }
 }