fix(importer): catch PIL OSError during transparency + phash blocks, skip as invalid_image instead of letting Celery autoretry loop forever
Operator hit a corrupt JPEG in the IR set 2026-05-25: PIL.verify() only validates header structure but doesn't catch truncated/broken pixel data. The error surfaces later in _transparency_pct (via getchannel 'A' -> load) or compute_phash (load) — both blow up with OSError 'broken data stream when reading image file'. Celery's autoretry_for then bounces the same file forever instead of marking it skipped. Wrap both PIL.load-triggering call sites with try/except OSError -> ImportResult(status=skipped, skip_reason=invalid_image). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -280,7 +280,18 @@ class Importer:
|
|||||||
)
|
)
|
||||||
|
|
||||||
if self.settings.skip_transparent and has_alpha:
|
if self.settings.skip_transparent and has_alpha:
|
||||||
pct = self._transparency_pct(source)
|
try:
|
||||||
|
pct = self._transparency_pct(source)
|
||||||
|
except OSError as exc:
|
||||||
|
# PIL.verify() at line 263 only validates header structure;
|
||||||
|
# truncated/corrupt pixel data only surfaces when load()
|
||||||
|
# actually decodes (here via getchannel('A')). Convert to
|
||||||
|
# invalid_image skip so the Celery autoretry loop doesn't
|
||||||
|
# bounce the same broken file forever.
|
||||||
|
return ImportResult(
|
||||||
|
status="skipped", skip_reason=SkipReason.invalid_image,
|
||||||
|
error=f"PIL load failed during transparency check: {exc}",
|
||||||
|
)
|
||||||
if pct >= self.settings.transparency_threshold:
|
if pct >= self.settings.transparency_threshold:
|
||||||
return ImportResult(
|
return ImportResult(
|
||||||
status="skipped", skip_reason=SkipReason.too_transparent,
|
status="skipped", skip_reason=SkipReason.too_transparent,
|
||||||
@@ -302,8 +313,16 @@ class Importer:
|
|||||||
# Perceptual near-dup (images only; videos keep phash NULL).
|
# Perceptual near-dup (images only; videos keep phash NULL).
|
||||||
phash = None
|
phash = None
|
||||||
if not is_video(source):
|
if not is_video(source):
|
||||||
with Image.open(source) as im:
|
try:
|
||||||
phash = compute_phash(im)
|
with Image.open(source) as im:
|
||||||
|
phash = compute_phash(im)
|
||||||
|
except OSError as exc:
|
||||||
|
# Same rationale as the transparency-check guard above:
|
||||||
|
# broken-pixel-data files pass verify() but blow up here.
|
||||||
|
return ImportResult(
|
||||||
|
status="skipped", skip_reason=SkipReason.invalid_image,
|
||||||
|
error=f"PIL load failed during phash compute: {exc}",
|
||||||
|
)
|
||||||
if phash is not None:
|
if phash is not None:
|
||||||
cand_rows = self.session.execute(
|
cand_rows = self.session.execute(
|
||||||
select(
|
select(
|
||||||
|
|||||||
@@ -134,3 +134,58 @@ def test_root_level_file_has_no_artist(importer, import_layout):
|
|||||||
importer.import_one(src)
|
importer.import_one(src)
|
||||||
artists = importer.session.execute(select(Artist)).scalars().all()
|
artists = importer.session.execute(select(Artist)).scalars().all()
|
||||||
assert artists == []
|
assert artists == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_pil_load_oserror_in_transparency_check_skips_not_raises(
|
||||||
|
importer, import_layout, monkeypatch,
|
||||||
|
):
|
||||||
|
"""PIL.verify() only validates header structure — broken pixel data
|
||||||
|
only surfaces when load() actually decodes. The importer must catch
|
||||||
|
the OSError and return a skipped: invalid_image result so the Celery
|
||||||
|
autoretry loop doesn't bounce the same broken file forever.
|
||||||
|
Operator hit this 2026-05-25 with a corrupt JPEG in the IR set."""
|
||||||
|
import_root, _ = import_layout
|
||||||
|
src = import_root / "Bob" / "corrupt.png"
|
||||||
|
# Make a real RGBA PNG so the has_alpha path engages.
|
||||||
|
_make_png_rgba(src, (100, 100), alpha=128)
|
||||||
|
|
||||||
|
importer.settings.skip_transparent = True
|
||||||
|
importer.settings.transparency_threshold = 0.5
|
||||||
|
|
||||||
|
# Force the next _transparency_pct call to raise as if PIL's load()
|
||||||
|
# blew up on truncated pixel data.
|
||||||
|
def _boom(_self, _src):
|
||||||
|
raise OSError("broken data stream when reading image file")
|
||||||
|
monkeypatch.setattr(
|
||||||
|
type(importer), "_transparency_pct", _boom,
|
||||||
|
)
|
||||||
|
|
||||||
|
result = importer.import_one(src)
|
||||||
|
assert result.status == "skipped"
|
||||||
|
assert result.skip_reason == SkipReason.invalid_image
|
||||||
|
assert "transparency check" in (result.error or "")
|
||||||
|
|
||||||
|
|
||||||
|
def test_pil_load_oserror_in_phash_compute_skips_not_raises(
|
||||||
|
importer, import_layout, monkeypatch,
|
||||||
|
):
|
||||||
|
"""Same shape as the transparency-check guard, but for the phash
|
||||||
|
compute block — the OTHER place PIL.load() runs implicitly during
|
||||||
|
the dedup pipeline."""
|
||||||
|
import_root, _ = import_layout
|
||||||
|
src = import_root / "Carol" / "corrupt.jpg"
|
||||||
|
_make_jpeg(src)
|
||||||
|
|
||||||
|
# Disable transparency check so we reach the phash compute block.
|
||||||
|
importer.settings.skip_transparent = False
|
||||||
|
|
||||||
|
from backend.app.services import importer as importer_module
|
||||||
|
|
||||||
|
def _boom(_im):
|
||||||
|
raise OSError("broken data stream when reading image file")
|
||||||
|
monkeypatch.setattr(importer_module, "compute_phash", _boom)
|
||||||
|
|
||||||
|
result = importer.import_one(src)
|
||||||
|
assert result.status == "skipped"
|
||||||
|
assert result.skip_reason == SkipReason.invalid_image
|
||||||
|
assert "phash compute" in (result.error or "")
|
||||||
|
|||||||
Reference in New Issue
Block a user