Remove image size limits.

[Zunhammer]

I suggest to remove the default image size limits from PIL. This causes training of models in nerfstudio to crash with the error:
DecompressionBombError: Image size (340840656 pixels) exceeds limit of 178956970 pixels, could be decompression
bomb DOS attack.

I see this is meant to be a security feature, however it prevents the usage of large images. I assume people are using mainly their own images in nerfstudio and don't see this as a security issue. Please let me know what you think about it.

Alternatively, there should be at least a param/option to allow large images.

I remove the image size limit at all occurances in the code where PIL is used, however this might not be necessary. E.g. the tests shouldn't be affected at all but I want to keep it consistent and avoid future issues.

Happy to get your feedback.

[jb-ye]

lgtm, @Zunhammer Needs to format the code.

[kerrj]

can we wrap the Image.MAX_IMAGE_PIXELS = None calls inside a helper util function? that way it can be toggled on/off in a single place in the code, and leaves more of a documentation trace as to what this is doing

[brentyi]

Thanks Nico for the PR! I agree with Justin that a function would be nice.

A context manager could make sense here, something like:

@contextlib.contextmanager
def set_pil_image_size_limit(max_pixels: Optional[int]):
    orig = Image.MAX_IMAGE_PIXELS
    Image.MAX_IMAGE_PIXELS = max_bytes
    yield
    Image.MAX_IMAGE_PIXELS = orig

which could be used to configure the setting only locally where it's needed.

It'd be nice to avoid the precedent of globally changing the behavior of other libraries when nerfstudio is imported, especially since it seems rare for users to have images that are >18000x18000 pixels.

[Zunhammer]

Thanks for the feedback, I see your point and agree. I'll change the PR later accordingly.

[Zunhammer]

@brentyi

As far as I see the value must be set after the import and cannot ne set by a function in another file as suggested. This also means a nerfstudio import woulnd't change any behaviour in other libraries/software. I suggest to globally define the default value
for the max pixels (None) and set this one on all files where PIL.Image is imported.

Please let me know if I misunderstand and implemented your suggestion wrong, or alternatively where I should globally define the MAX_IMAGE_PIXELS default.

Thanks

[brentyi]

if we define set_pil_image_size_limit() as a context manager, it needs to be used in a with statement

there's an example in the Python docs here: https://docs.python.org/3/library/contextlib.html#contextlib.contextmanager

[brentyi]

This also means a nerfstudio import woulnd't change any behaviour in other libraries/software. I suggest to globally define the default value for the max pixels (None) and set this one on all files where PIL.Image is imported.

Sorry for the delayed response here — is this something you observed empirically? This contradicts my understanding of Python semantics: if you assign Image.MAX_IMAGE_PIXELS = None from anywhere (eg just once), then Image.MAX_IMAGE_PIXELS should resolve to None from everywhere Image is imported.

To clarify my set_pil_image_size_limit() context manager suggestion, the intention was to provide a helper for only locally setting Image.MAX_IMAGE_PIXELS. Then you could write:

with set_pil_image_size_limit(None):
    Image.open(...)

where the MAX_IMAGE_PIXELS constant is reassigned at the start of the with block, then reverted to its original value at the end of the block 🙂

[Zunhammer]

Hey Brent,

thanks for the reply. I'll adjust and test again soon. Thanks for the input :)