thumbor / thumbor-plugins / 40602191909

99%

push

github

fix(optipng): run optipng without a shell, coerce the level, add a timeout (#121)

The optimizer ran optipng 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 rest
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. OPTIPNG_LEVEL was formatted with %d, so the
string value that a thumbor.conf written like the README examples produces
raised TypeError on every PNG request. There was no timeout either, and
with the default ENGINE_THREADPOOL_SIZE of 0 a slow run blocks the event
loop.

optipng now runs as an argument list under OPTIPNG_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. OPTIPNG_LEVEL
and OPTIPNG_TIMEOUT are converted when the optimizer is built, since both can
arrive as strings from thumbor.conf or --use-environment. A level that is not
an integer of at least 1 (-o0 with -force exits 1) or a timeout that is
negative, infinite or not a number disables the optimizer with an error, like a
missing binary does; levels above 7 are passed as 7, which optipng treats
identically. -quiet is gone so the warning carries optipng's diagnostics, and
the <output>.bak copy that -keep left behind on every request is removed. 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.

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

266 of 268 relevant lines covered (99.25%)

0.99 hits per line