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

hint to how to suppress decompressionbomb warnings #235

Merged
merged 1 commit into from Jan 25, 2017

Conversation

2 participants
@t-animal
Contributor

t-animal commented Jan 20, 2017

The warnings could also be suppressed by wrapping PILImage.open() in Image with warnings.catch_warnings:

with warnings.catch_warnings():
    img = PILImage.open(...)

However that is not defined behaviour for multithreading: https://docs.python.org/2/library/warnings.html#temporarily-suppressing-warnings

@saimn

This comment has been minimized.

Owner

saimn commented Jan 20, 2017

Using catch_warnings for the PILImage.open calls seems more sensible, it should also catch only the DecompressionBombWarning (warnings.simplefilter('ignore', Image.DecompressionBombWarning)).
I could also be a good idea to log a message when this happens, not sure about the level, DEBUG or INFO ?

@t-animal t-animal force-pushed the t-animal:warnings_ignore branch from 75473b5 to 6074f14 Jan 20, 2017

@t-animal

This comment has been minimized.

Contributor

t-animal commented Jan 20, 2017

I updated the filter.

I don't know if it's a good idea to wrap all PILImage.open calls in a catch_warnings context, as the documentation states that "If two or more threads use the catch_warnings context manager at the same time, the behavior is undefined". I guess at least generate_image is called in a multithreading environment.

By default, there is a log message, because the warnings module logs to stderr.

@saimn

This comment has been minimized.

Owner

saimn commented Jan 20, 2017

Sigal is using multiple processes, not threads, so it should be fine.

By default, there is a log message, because the warnings module logs to stderr.

But not if you catch the warning. So my idea is that it should still be possible to see the warning message in debug/verbose mode, but not in the normal mode.

@t-animal t-animal force-pushed the t-animal:warnings_ignore branch from 6074f14 to 8851d31 Jan 21, 2017

@t-animal

This comment has been minimized.

Contributor

t-animal commented Jan 21, 2017

Unfortunately one cannot use the context manager to filter for the category, so I did that manually. I logged the warnings to info, as it might be interesting to a user, not just a developer.

@t-animal t-animal force-pushed the t-animal:warnings_ignore branch from 8851d31 to 510f20f Jan 21, 2017

@saimn saimn merged commit c2621dc into saimn:master Jan 25, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@saimn

This comment has been minimized.

Owner

saimn commented Jan 25, 2017

Unfortunately one cannot use the context manager to filter for the category, so I did that manually

Yep, right. Thanks !

@t-animal t-animal deleted the t-animal:warnings_ignore branch Jan 26, 2017

@saimn saimn added this to the 1.4.0 milestone Feb 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment