thumbor / thumbor-plugins / 40602141291

99%

push

github

fix(pngquant): run pngquant without a shell and survive failures (#122)

The optimizer built a "cat input | pngquant ... > output" string and ran
it through subprocess.call(shell=True), ignoring the exit status. Nothing
from the request reaches that string today (thumbor supplies the temp file
names and the other values come from thumbor.conf), but the shell was
unnecessary and any failure left output_file empty. thumbor serves whatever
is in that file, so an empty file turned into a 500 (no Content-Type can be
derived from zero bytes) instead of the unoptimized PNG. The process also
had no timeout, and with thumbor's default ENGINE_THREADPOOL_SIZE of 0 a
slow run blocks the whole event loop.

pngquant now runs as an argument list with stdin and stdout bound to the
temp files, under a new PNGQUANT_TIMEOUT (default 30 seconds, 0 disables).
A non-zero exit, an empty output or a timeout logs a warning and copies the
input over the output so thumbor serves the original image; exit status 99
(quality floor not met) is logged at debug level. PNGQUANT_QUALITY (max,
min-max, -max or min- within 0-100), PNGQUANT_SPEED (1 to 11, passed
without the leading zeros pngquant rejects) and PNGQUANT_TIMEOUT (finite,
0 or more) are checked when the optimizer is built, since they arrive as
strings from thumbor.conf or --use-environment, and an invalid value
disables it with an error instead of reaching pngquant. Unit tests use a
real temp directory and a fake subprocess.run instead of patching
os.path.isfile and os.access, whose tearDown stopped the os.access mock
rather than its patcher, so that patch was never undone.

47 of 47 new or added lines in 2 files covered. (100.0%)

148 of 148 relevant lines covered (100.0%)

1.0 hits per line