ImageSharp icon indicating copy to clipboard operation
ImageSharp copied to clipboard

Apply `AllocationLimitMegabytes` against the total size of multi-frame images before attempting to decode them

Open antonfirsov opened this issue 11 months ago • 2 comments

Discussed in https://github.com/SixLabors/ImageSharp/discussions/2849

Originally posted by antonfirsov December 17, 2024

Configuration.Default.MemoryAllocator = MemoryAllocator.Create(new MemoryAllocatorOptions()
{
    AllocationLimitMegabytes = 16
});

// Should throw InvalidMemoryOperationException when the total size of all frames
// to allocate for this image would be over 16MB, even if the individual frame sizes are below 16MB.
var img = Image.Load("multiframe.gif");

Even though strictly speaking this would be a hack since the allocation limit is originally meant to be per-buffer, #2848 made me think this is something users want us to do. For built-in decoders, this is possible to implement by adding an internal validation method toMemoryAllocator; we can figure out later whether we want to make the feature available to external decoders.

antonfirsov avatar Dec 17 '24 03:12 antonfirsov

I think we should avoid the hack and add a new property.

  • AllocationLimitMegabytes
  • AccumulativeAllocationLimitMegabytes

I thought all allocators must inherit MemoryAllocator; can we not add tracking there? I'm happy to break the inheritance pattern to do this.

JimBobSquarePants avatar Dec 18 '24 01:12 JimBobSquarePants

The problem with an extra property is that it adds complexity while I find it unlikely that users would want to configure the values separately; or do you see use cases where they would?

I thought all allocators must inherit MemoryAllocator

They all do. Note that the limit properties live in MemoryAllocatorOptions.

antonfirsov avatar Dec 18 '24 15:12 antonfirsov