-
Notifications
You must be signed in to change notification settings - Fork 27.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adds checks for resulting image size to avoid memory issues #8175
Conversation
I would suggest this be added to common code, Last week I ran into this error without using the xyz grid script, when I did an extremely large batch (800 images, 858MB, 37120x20160, 748.34 Megapixel Grid Image) and attempted to zip the result. In many ways, I think it would also be useful if the grid image could just be split every 100 images or so, creating multiple grids for extra large batches, as well as an option to skip zipping the grid image itself when zipping batches. |
fair point. i'll take a look at that as well, but lets get this comited first. |
The intended approach is to throw rather than return something. Do this with |
I modified image.py to generate big grid images :D |
done. |
What's with this change? The error message is twice the set value and even generating a 2x1 64px grid far exceeds the default value. DecompressionBombError: Image size (4096 pixels) exceeds limit of 440.00000000000006 pixels, could be decompression bomb DOS attack. |
@@ -484,6 +484,11 @@ def process_axis(opt, vals): | |||
z_opt = self.current_axis_options[z_type] | |||
zs = process_axis(z_opt, z_values) | |||
|
|||
# this could be moved to common code, but unlikely to be ever triggered anywhere else | |||
Image.MAX_IMAGE_PIXELS = opts.img_max_size_mp * 1.1 # allow 10% overhead for margins and legend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field set should not be here.
... A, in the first place at all, but also B the unit is different (pixels vs megapixels)
The field is used in PIL/Image.py
to check for decompression bomb attacks which is definitely not relevant here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal is to avoid Pillow decompression bomb runtime errors that nobody understood and was creating issues and replace them with easily understandable and configurable option. But latest update of the PR created a typo, fixed.
I set MAX_IMAGE_PIXELS to None in PIL/Image.py :), that's work |
adds checks for resulting image size to avoid memory issues
adds field in Settings -> Saving Images/Grids -> Maximum image size, in megapixels
(implemented as
shared.opts.img_max_size_mp
)enforces a check inside xyz grid script as that is common place where this issue occurs.
on error, results will be none and error will be printed in webui (using infotext).
closes #8051
closes #8164