diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs index f0cc4d5e82..3bdaf2dc27 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs @@ -16,7 +16,7 @@ public ComponentProcessor(MemoryAllocator memoryAllocator, JpegFrame frame, Size this.Component = component; this.BlockAreaSize = component.SubSamplingDivisors * blockSize; - this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( + this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, this.BlockAreaSize.Height); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs index c33a8a1968..9346b11e7b 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs @@ -28,7 +28,7 @@ public ComponentProcessor(MemoryAllocator memoryAllocator, Component component, this.blockAreaSize = component.SubSamplingDivisors * 8; // alignment of 8 so each block stride can be sampled from a single 'ref pointer' - this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( + this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, 8, diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 2bd9cb5eef..341dacb38d 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -20,6 +20,18 @@ public abstract class MemoryAllocator /// public static MemoryAllocator Default { get; } = Create(); + /// + /// Gets or sets the maximum allowable allocatable size of a 2 dimensional buffer. + /// Defaults to 65535 * 65535. + /// + public Size MaxAllocatableSize2D { get; set; } = new Size(ushort.MaxValue, ushort.MaxValue); + + /// + /// Gets or sets the maximum allowable allocatable size of a 1 dimensional buffer. + /// + /// Defaults to 1073741823. + public int MaxAllocatableSize1D { get; set; } = int.MaxValue / 2; + /// /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes. /// @@ -42,7 +54,7 @@ public static MemoryAllocator Create(MemoryAllocatorOptions options) => new UniformUnmanagedMemoryPoolMemoryAllocator(options.MaximumPoolSizeMegabytes); /// - /// Allocates an , holding a of length . + /// Allocates an , holding a of length . /// /// Type of the data stored in the buffer. /// Size of the buffer to allocate. @@ -64,6 +76,7 @@ public virtual void ReleaseRetainedResources() /// /// Allocates a . /// + /// Type of the data stored in the buffer. /// The total length of the buffer. /// The expected alignment (eg. to make sure image rows fit into single buffers). /// The . @@ -75,4 +88,14 @@ internal virtual MemoryGroup AllocateGroup( AllocationOptions options = AllocationOptions.None) where T : struct => MemoryGroup.Allocate(this, totalLength, bufferAlignment, options); + + internal static void MemoryGuardMustBeBetweenOrEqualTo(int value, int min, int max, string paramName) + { + if (value >= min && value <= max) + { + return; + } + + throw new InvalidMemoryOperationException($"Parameter \"{paramName}\" must be between or equal to {min} and {max}, was {value}"); + } } diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index 41730d9678..f979ebf000 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. using System.Buffers; @@ -7,7 +7,7 @@ namespace SixLabors.ImageSharp.Memory; /// -/// Implements by newing up managed arrays on every allocation request. +/// Implements by creating new managed arrays on every allocation request. /// public sealed class SimpleGcMemoryAllocator : MemoryAllocator { @@ -17,7 +17,7 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); + MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); return new BasicArrayBuffer(new T[length]); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 0bae193632..e410a0dbca 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -79,16 +79,14 @@ internal UniformUnmanagedMemoryPoolMemoryAllocator( protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; /// - public override IMemoryOwner Allocate( - int length, - AllocationOptions options = AllocationOptions.None) + public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); + MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); int lengthInBytes = length * Unsafe.SizeOf(); if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) { - var buffer = new SharedArrayPoolBuffer(length); + SharedArrayPoolBuffer buffer = new(length); if (options.Has(AllocationOptions.Clean)) { buffer.GetSpan().Clear(); @@ -102,8 +100,7 @@ public override IMemoryOwner Allocate( UnmanagedMemoryHandle mem = this.pool.Rent(); if (mem.IsValid) { - UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, length, options.Has(AllocationOptions.Clean)); - return buffer; + return this.pool.CreateGuardedBuffer(mem, length, options.Has(AllocationOptions.Clean)); } } @@ -124,7 +121,7 @@ internal override MemoryGroup AllocateGroup( if (totalLengthInBytes <= this.sharedArrayPoolThresholdInBytes) { - var buffer = new SharedArrayPoolBuffer((int)totalLength); + SharedArrayPoolBuffer buffer = new((int)totalLength); return MemoryGroup.CreateContiguous(buffer, options.Has(AllocationOptions.Clean)); } diff --git a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs index da202aa596..62e14affba 100644 --- a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs @@ -20,7 +20,9 @@ internal class UnmanagedMemoryAllocator : MemoryAllocator public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - var buffer = UnmanagedBuffer.Allocate(length); + MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); + + UnmanagedBuffer buffer = UnmanagedBuffer.Allocate(length); if (options.Has(AllocationOptions.Clean)) { buffer.GetSpan().Clear(); diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index ff306e1e45..42359d029f 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -18,20 +18,23 @@ public static class MemoryAllocatorExtensions /// The memory allocator. /// The buffer width. /// The buffer height. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, int width, int height, - bool preferContiguosImageBuffers, + bool preferContiguousImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct { + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); + long groupLength = (long)width * height; MemoryGroup memoryGroup; - if (preferContiguosImageBuffers && groupLength < int.MaxValue) + if (preferContiguousImageBuffers && groupLength < int.MaxValue) { IMemoryOwner buffer = memoryAllocator.Allocate((int)groupLength, options); memoryGroup = MemoryGroup.CreateContiguous(buffer, false); @@ -69,16 +72,16 @@ public static Buffer2D Allocate2D( /// The type of buffer items to allocate. /// The memory allocator. /// The buffer size. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, Size size, - bool preferContiguosImageBuffers, + bool preferContiguousImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct => - Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguosImageBuffers, options); + Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguousImageBuffers, options); /// /// Allocates a buffer of value type objects interpreted as a 2D region @@ -96,7 +99,7 @@ public static Buffer2D Allocate2D( where T : struct => Allocate2D(memoryAllocator, size.Width, size.Height, false, options); - internal static Buffer2D Allocate2DOveraligned( + internal static Buffer2D Allocate2DOverAligned( this MemoryAllocator memoryAllocator, int width, int height, @@ -104,6 +107,9 @@ internal static Buffer2D Allocate2DOveraligned( AllocationOptions options = AllocationOptions.None) where T : struct { + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); + long groupLength = (long)width * height; MemoryGroup memoryGroup = memoryAllocator.AllocateGroup( groupLength, @@ -127,6 +133,8 @@ internal static IMemoryOwner AllocatePaddedPixelRowBuffer( int paddingInBytes) { int length = (width * pixelSizeInBytes) + paddingInBytes; + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(length, 0, memoryAllocator.MaxAllocatableSize1D, nameof(length)); + return memoryAllocator.Allocate(length); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs index c1907bb520..da26cbefcd 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs @@ -50,7 +50,7 @@ private ResizeKernelMap( this.sourceLength = sourceLength; this.DestinationLength = destinationLength; this.MaxDiameter = (radius * 2) + 1; - this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguosImageBuffers: true, AllocationOptions.Clean); + this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguousImageBuffers: true, AllocationOptions.Clean); this.pinHandle = this.data.DangerousGetSingleMemory().Pin(); this.kernels = new ResizeKernel[destinationLength]; this.tempValues = new double[this.MaxDiameter]; diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index cce27a401c..fd13d7b5a6 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -83,7 +83,7 @@ public ResizeWorker( this.transposedFirstPassBuffer = configuration.MemoryAllocator.Allocate2D( this.workerHeight, targetWorkingRect.Width, - preferContiguosImageBuffers: true, + preferContiguousImageBuffers: true, options: AllocationOptions.Clean); this.tempRowBuffer = configuration.MemoryAllocator.Allocate(this.sourceRectangle.Width); diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index 78a7b2c11c..f619e37cad 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -558,4 +558,13 @@ public void BmpDecoder_CanDecode_Os2BitmapArray(TestImageProvider(TestImageProvider provider) + where TPixel : unmanaged, IPixel + => Assert.Throws(() => + { + using Image image = provider.GetImage(BmpDecoder.Instance); + }); } diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs index 42cbd90f3b..3d6aab9bcd 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs @@ -3,6 +3,7 @@ using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Bmp; +using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; @@ -349,13 +350,15 @@ public void Encode_PreservesColorProfile(TestImageProvider provi } [Theory] - [InlineData(1, 66535)] - [InlineData(66535, 1)] + [InlineData(1, 65536)] + [InlineData(65536, 1)] public void Encode_WorksWithSizeGreaterThen65k(int width, int height) { Exception exception = Record.Exception(() => { - using Image image = new Image(width, height); + Configuration c = Configuration.CreateDefaultInstance(); + c.MemoryAllocator = new UniformUnmanagedMemoryPoolMemoryAllocator(null) { MaxAllocatableSize2D = new(65537, 65537) }; + using Image image = new Image(c, width, height); using MemoryStream memStream = new(); image.Save(memStream, BmpEncoder); }); diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs index 780ba7f20e..b00dfa392a 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs @@ -21,11 +21,9 @@ public BufferTests() [Theory] [InlineData(-1)] + [InlineData(1073741823 + 1)] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) - { - ArgumentOutOfRangeException ex = Assert.Throws(() => this.MemoryAllocator.Allocate(length)); - Assert.Equal("length", ex.ParamName); - } + => Assert.Throws(() => this.MemoryAllocator.Allocate(length)); [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 7e7c136635..d6ccfa8399 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -114,6 +114,12 @@ public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperati Assert.Throws(() => allocator.AllocateGroup(int.MaxValue * (long)int.MaxValue, int.MaxValue)); } + [Theory] + [InlineData(-1)] + [InlineData(1073741823 + 1)] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) + => Assert.Throws(() => new UniformUnmanagedMemoryPoolMemoryAllocator(null).Allocate(length)); + [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() { diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 5364de0652..a73a3c23ec 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -72,11 +72,11 @@ public void Construct_PreferContiguousImageBuffers_AllocatesContiguousRegardless using Buffer2D buffer = useSizeOverload ? this.MemoryAllocator.Allocate2D( new Size(200, 200), - preferContiguosImageBuffers: true) : + preferContiguousImageBuffers: true) : this.MemoryAllocator.Allocate2D( 200, 200, - preferContiguosImageBuffers: true); + preferContiguousImageBuffers: true); Assert.Equal(1, buffer.FastMemoryGroup.Count); Assert.Equal(200 * 200, buffer.FastMemoryGroup.TotalLength); } @@ -87,7 +87,7 @@ public void Allocate2DOveraligned(int bufferCapacity, int width, int height, int { this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2DOveraligned(width, height, alignmentMultiplier); + using Buffer2D buffer = this.MemoryAllocator.Allocate2DOverAligned(width, height, alignmentMultiplier); MemoryGroup memoryGroup = buffer.FastMemoryGroup; int expectedAlignment = width * alignmentMultiplier; @@ -337,4 +337,22 @@ public void PublicMemoryGroup_IsMemoryGroupView() Assert.False(mgBefore.IsValid); Assert.NotSame(mgBefore, buffer1.MemoryGroup); } + + [Theory] + [InlineData(-1)] + [InlineData(ushort.MaxValue + 1)] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(length, length)); + + [Theory] + [InlineData(-1)] + [InlineData(ushort.MaxValue + 1)] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_Size(int length) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(length, length))); + + [Theory] + [InlineData(-1)] + [InlineData(ushort.MaxValue + 1)] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_OverAligned(int length) + => Assert.Throws(() => this.MemoryAllocator.Allocate2DOverAligned(length, length, 1)); } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 5da581e52f..d83abc52f3 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -442,6 +442,8 @@ public static class Bmp public const string BlackWhitePalletDataMatrix = "Bmp/bit1datamatrix.bmp"; + public const string Issue2696 = "Bmp/issue-2696.bmp"; + public static readonly string[] BitFields = { Rgb32bfdef, diff --git a/tests/Images/Input/Bmp/issue-2696.bmp b/tests/Images/Input/Bmp/issue-2696.bmp new file mode 100644 index 0000000000..6770dd9469 --- /dev/null +++ b/tests/Images/Input/Bmp/issue-2696.bmp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:bc42cda9bac8fc73351ad03bf55214069bb8d31ea5bdd806321a8cc8b56c282e +size 126