Limit decoded images to 2^26 pixels by default

[zealousidealroll]

Fixes #80

[bvssvni]

Looks good to me.

[HeroicKatora]

The limit appears somewhat arbitrary to me. Is it referring to a limit already established in another library?

[HeroicKatora]

Considering that it triggers on a quadratic image of width and length each 2^13 = 8192 which is already well in range of some of the better cameras (current DSLR having 20-40M at the moment) and might even become a monitor resolution in the foreseeable future. For example, IMAX is estimated at just about 10000x7000 according to Wikipedia. And Japan wants to record the 2020 olympics in 16k resolution (close to 132M).

[oyvindln]

In the BMP decoder in image there is a max of 2^16, though one may not expect bmp images to be quite as large as a png image (as they are not, or only very slightly compressed).

There is also a limit on how much memory the decoder allocates at a first try, and it will only allocate more if there is actually image data for it. Maybe something similar could be done here.

Ideally though it would be nice if a limit could be specified, so the caller could give some reasonable expected max size based on say the file size.

[HeroicKatora]

The max in BMP is enforced for width and height individually, so in total limit the size to something slightly smaller than 2^32. The limit proposed here is for the actual pixel count, width*height.

The suggestion on additional memory optimization sounds sensible, so like an adjusted read_to_end where we have a slightly different heuristic because we expect the full image instead of an unknown amount of data.

[oyvindln]

Yeah I meant to write 2^16 in one direction but I somehow missed it.

[zealousidealroll]

The limit was copied from the flif crate.

[HeroicKatora]

Ah, I didn't realize that. In the corresponding pull request, the choice was explained as follows:

Default values allow maximum size of DecodingImage's data equal to 2^26, which is equal to 512 MB.

Now I wonder if we should differentiate between a pixel and a memory limit. png allows bit depths of up to 16 bit and up to 4 samples per pixel, leading to up to 16=2^4 bytes per pixel in the most extreme case. Setting a limit on the number of pixels would enforce somewhere between 32MiBi and 1024MiBi of memory usage, depending on the colour format.

However, that is more of a bikeshedding issue for now, with prior art demonstrated it might be sensible to just merge it for now. Increasing the default limit in the future or adding alternative and optional additional limits should not be a breaking change anyways.

[newpavlov]

I think there shouldn't be any problems with making pixels field public. This will also allow us to remove set_pixels setter.

[newpavlov]

How about using new_with_limits(r: R, limits: Limits) method instead? It will allow to create decoder in one line and new can be defined as Self::new_with_limits(r, Default::default()).

[Shnatsel]

For reference, libpng enforces 1,000,000 by 1,000,000 pixels limit by default, which is roughly 2^40 pixels or 2^44 bytes per image in the extreme case.

Source and further information: https://libpng.sourceforge.io/decompression_bombs.html

[zealousidealroll]

2^40 pixels

• Even with a measly 256 colors (so it would be 2^40 bytes), that's bigger than the entire address space on a 32-bit system. If nothing else, the limits should never be higher than 2^27 on a 32-bit system (making 2^31 bytes at 2^4=16 bytes per pixel, which will exactly fill the userland-accessible address space on a 32-bit system where the upper half is reserved for the operating system). If you think anyone's going to need higher, then it should probably be conditional on the platform.

• According to my quick Wikipedia visit, an 8K picture is 2^25 pixels. In the immortal words of Bill Gates, 8K should be enough for anybody.

[bvssvni]

Merging.