thumbor / thumbor-plugins / 40602420937

99%

push

github

fix(pngcrush): run pngcrush without a shell and survive failures (#123)

The optimizer ran pngcrush through subprocess.call(shell=True) with a formatted
string and ignored the exit status. Nothing from the request reaches that
string (thumbor supplies the temp file names and the binary path comes from
thumbor.conf), but the shell was unnecessary, a failure left output_file empty,
and thumbor turns an empty output into a 500 instead of serving the unoptimized
PNG. There was no timeout either, and with the default ENGINE_THREADPOOL_SIZE
of 0 a slow run blocks the event loop.

pngcrush now runs as an argument list with the same flags, under
PNGCRUSH_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; pngcrush exits 0 even when it fails, so the empty-
output case gets its own message. The timeout is converted when the optimizer
is built, since a quoted thumbor.conf value or --use-environment yields a
string, and a negative, infinite or non-numeric value disables the optimizer
with an error. Unit tests use a real temp directory and a fake subprocess.run
instead of patching os.path.isfile and os.access; tearDown stopped the
os.access mock rather than its patcher, so that patch was never undone.

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

288 of 290 relevant lines covered (99.31%)

0.99 hits per line