thumbor / thumbor-plugins / 40602467120

99%

push

github

fix(jpegrecompress): run jpeg-recompress without a shell, survive failures (#124)

The optimizer ran jpeg-recompress 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 and a failure left
output_file empty, which thumbor turns into a 500 instead of the unoptimized
JPEG. jpeg-recompress can exit 1 without writing output when its final encode
is not smaller than the input, so this was reachable with valid input (a small
flat-color JPEG served with PROGRESSIVE_JPEG = False reproduces it). There was
no timeout either, and --accurate --loops 10 is its slow mode; with the default
ENGINE_THREADPOOL_SIZE of 0 a slow run blocks the event loop.

jpeg-recompress now runs as an argument list with the same options plus
--quiet, which only silences the per-iteration progress lines so the warning
carries the actual error, under JPEGRECOMPRESS_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. 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. The unused PIL import is gone, and
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.

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

305 of 307 relevant lines covered (99.35%)

0.99 hits per line