From 9f04434804a94199d5321ed380ec21104027056e Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 4 Jan 2023 11:57:18 +0100 Subject: [PATCH 1/2] Add byteLength to WrapMemory * Added byteLength parameter to WrapMemory method and check that the memory is at least the size of byteLength --- src/ImageSharp/Image.WrapMemory.cs | 12 +++++- .../Image/ImageTests.WrapMemory.cs | 42 +++++++++++++++---- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index 0c9e14896..5fe4c8049 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -403,6 +403,7 @@ public abstract partial class Image /// The pixel type /// The /// The pointer to the target memory buffer to wrap. + /// The byte length of the memory allocated. /// The width of the memory image. /// The height of the memory image. /// The . @@ -412,6 +413,7 @@ public abstract partial class Image public static unsafe Image WrapMemory( Configuration configuration, void* pointer, + int byteLength, int width, int height, ImageMetadata metadata) @@ -423,6 +425,8 @@ public abstract partial class Image UnmanagedMemoryManager memoryManager = new(pointer, width * height); + Guard.MustBeGreaterThan(byteLength, memoryManager.Memory.Span.Length, nameof(byteLength)); + MemoryGroup memorySource = MemoryGroup.Wrap(memoryManager.Memory); return new Image(configuration, memorySource, width, height, metadata); } @@ -453,6 +457,7 @@ public abstract partial class Image /// The pixel type /// The /// The pointer to the target memory buffer to wrap. + /// The byte length of the memory allocated. /// The width of the memory image. /// The height of the memory image. /// The configuration is null. @@ -460,10 +465,11 @@ public abstract partial class Image public static unsafe Image WrapMemory( Configuration configuration, void* pointer, + int byteLength, int width, int height) where TPixel : unmanaged, IPixel - => WrapMemory(configuration, pointer, width, height, new ImageMetadata()); + => WrapMemory(configuration, pointer, byteLength, width, height, new ImageMetadata()); /// /// @@ -490,13 +496,15 @@ public abstract partial class Image /// /// The pixel type. /// The pointer to the target memory buffer to wrap. + /// The byte length of the memory allocated. /// The width of the memory image. /// The height of the memory image. /// An instance. public static unsafe Image WrapMemory( void* pointer, + int byteLength, int width, int height) where TPixel : unmanaged, IPixel - => WrapMemory(Configuration.Default, pointer, width, height); + => WrapMemory(Configuration.Default, pointer, byteLength, width, height); } diff --git a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs index 1b2c20548..b62f3ebd8 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.WrapMemory.cs @@ -108,7 +108,8 @@ public partial class ImageTests if (remainder != 0) { - ThrowHelper.ThrowArgumentException("The input index doesn't result in an aligned item access", nameof(elementIndex)); + ThrowHelper.ThrowArgumentException("The input index doesn't result in an aligned item access", + nameof(elementIndex)); } return this.memory.Slice(shiftedOffset).Pin(); @@ -269,7 +270,8 @@ public partial class ImageTests // To check that the underlying data matches, we can just manually check their lenth, and the // fact that a reference to the first pixel in both spans is actually the same memory location. Assert.Equal(pixelSpan.Length, imageSpan.Length); - Assert.True(Unsafe.AreSame(ref pixelSpan.GetPinnableReference(), ref imageSpan.GetPinnableReference())); + Assert.True(Unsafe.AreSame(ref pixelSpan.GetPinnableReference(), + ref imageSpan.GetPinnableReference())); image.GetPixelMemoryGroup().Fill(bg); image.ProcessPixelRows(accessor => @@ -292,6 +294,30 @@ public partial class ImageTests } } + + [Fact] + public unsafe void WrapMemory_Throws_OnTooLessWrongSize() + { + var cfg = Configuration.CreateDefaultInstance(); + var metaData = new ImageMetadata(); + + var array = new Rgba32[25]; + Exception thrownException = null; + fixed (void* ptr = array) + { + try + { + using (var image = Image.WrapMemory(cfg, ptr, 24, 5, 5, metaData)); + } + catch (Exception e) + { + thrownException = e; + } + } + + Assert.IsType(thrownException); + } + [Fact] public unsafe void WrapMemory_FromPointer_CreatedImageIsCorrect() { @@ -302,7 +328,7 @@ public partial class ImageTests fixed (void* ptr = array) { - using (var image = Image.WrapMemory(cfg, ptr, 5, 5, metaData)) + using (var image = Image.WrapMemory(cfg, ptr, 25, 5, 5, metaData)) { Assert.True(image.DangerousTryGetSinglePixelMemory(out Memory imageMem)); Span imageSpan = imageMem.Span; @@ -335,13 +361,14 @@ public partial class ImageTests fixed (void* p = pixelMemory.Span) { - using (var image = Image.WrapMemory(p, bmp.Width, bmp.Height)) + using (var image = Image.WrapMemory(p, pixelMemory.Length, bmp.Width, bmp.Height)) { Span pixelSpan = pixelMemory.Span; Span imageSpan = image.GetRootFramePixelBuffer().DangerousGetSingleMemory().Span; Assert.Equal(pixelSpan.Length, imageSpan.Length); - Assert.True(Unsafe.AreSame(ref pixelSpan.GetPinnableReference(), ref imageSpan.GetPinnableReference())); + Assert.True(Unsafe.AreSame(ref pixelSpan.GetPinnableReference(), + ref imageSpan.GetPinnableReference())); image.GetPixelMemoryGroup().Fill(bg); image.ProcessPixelRows(accessor => @@ -527,10 +554,11 @@ public partial class ImageTests [InlineData(1023, 32, 32)] public unsafe void WrapMemory_Pointer_Null(int size, int height, int width) { - Assert.Throws(() => Image.WrapMemory((void*)null, height, width)); + Assert.Throws(() => Image.WrapMemory((void*)null, size, height, width)); } private static bool ShouldSkipBitmapTest => - !TestEnvironment.Is64BitProcess || (TestHelpers.ImageSharpBuiltAgainst != "netcoreapp3.1" && TestHelpers.ImageSharpBuiltAgainst != "netcoreapp2.1"); + !TestEnvironment.Is64BitProcess || (TestHelpers.ImageSharpBuiltAgainst != "netcoreapp3.1" && + TestHelpers.ImageSharpBuiltAgainst != "netcoreapp2.1"); } } From 92ea71d155a40aee2acac59d4a45251aff9bbcf7 Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 4 Jan 2023 13:11:55 +0100 Subject: [PATCH 2/2] Use MustBeGreaterThanOrEqualTo instead of MustBeGreaterThan * Rename parameter --- src/ImageSharp/Image.WrapMemory.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ImageSharp/Image.WrapMemory.cs b/src/ImageSharp/Image.WrapMemory.cs index 5fe4c8049..d8cea246f 100644 --- a/src/ImageSharp/Image.WrapMemory.cs +++ b/src/ImageSharp/Image.WrapMemory.cs @@ -403,7 +403,7 @@ public abstract partial class Image /// The pixel type /// The /// The pointer to the target memory buffer to wrap. - /// The byte length of the memory allocated. + /// The byte length of the memory allocated. /// The width of the memory image. /// The height of the memory image. /// The . @@ -413,7 +413,7 @@ public abstract partial class Image public static unsafe Image WrapMemory( Configuration configuration, void* pointer, - int byteLength, + int bufferSizeInBytes, int width, int height, ImageMetadata metadata) @@ -425,7 +425,7 @@ public abstract partial class Image UnmanagedMemoryManager memoryManager = new(pointer, width * height); - Guard.MustBeGreaterThan(byteLength, memoryManager.Memory.Span.Length, nameof(byteLength)); + Guard.MustBeGreaterThanOrEqualTo(bufferSizeInBytes, memoryManager.Memory.Span.Length, nameof(bufferSizeInBytes)); MemoryGroup memorySource = MemoryGroup.Wrap(memoryManager.Memory); return new Image(configuration, memorySource, width, height, metadata); @@ -457,7 +457,7 @@ public abstract partial class Image /// The pixel type /// The /// The pointer to the target memory buffer to wrap. - /// The byte length of the memory allocated. + /// The byte length of the memory allocated. /// The width of the memory image. /// The height of the memory image. /// The configuration is null. @@ -465,11 +465,11 @@ public abstract partial class Image public static unsafe Image WrapMemory( Configuration configuration, void* pointer, - int byteLength, + int bufferSizeInBytes, int width, int height) where TPixel : unmanaged, IPixel - => WrapMemory(configuration, pointer, byteLength, width, height, new ImageMetadata()); + => WrapMemory(configuration, pointer, bufferSizeInBytes, width, height, new ImageMetadata()); /// /// @@ -496,15 +496,15 @@ public abstract partial class Image /// /// The pixel type. /// The pointer to the target memory buffer to wrap. - /// The byte length of the memory allocated. + /// The byte length of the memory allocated. /// The width of the memory image. /// The height of the memory image. /// An instance. public static unsafe Image WrapMemory( void* pointer, - int byteLength, + int bufferSizeInBytes, int width, int height) where TPixel : unmanaged, IPixel - => WrapMemory(Configuration.Default, pointer, byteLength, width, height); + => WrapMemory(Configuration.Default, pointer, bufferSizeInBytes, width, height); }