fix: a downloaded file with broken pixel data is skipped instead of ending the download run
CI and images / lint (push) Successful in 2s
CI and images / extension-version (push) Successful in 2s
CI and images / frontend-build (push) Successful in 24s
CI and images / extension-test (push) Successful in 19s
CI and images / backend-lint-and-test (push) Successful in 46s
CI and images / integration (push) Successful in 2m47s
CI and images / sign-extension (push) Successful in 2s
CI and images / build-agent (push) Successful in 4s
CI and images / build-web (push) Successful in 1m59s
CI and images / smoke-web (push) Successful in 49s
CI and images / promote (push) Successful in 1s
CI and images / refresh-dev (push) Skipped
CI and images / unattended-failure (push) Skipped
CI and images / lint (push) Successful in 2s
CI and images / extension-version (push) Successful in 2s
CI and images / frontend-build (push) Successful in 24s
CI and images / extension-test (push) Successful in 19s
CI and images / backend-lint-and-test (push) Successful in 46s
CI and images / integration (push) Successful in 2m47s
CI and images / sign-extension (push) Successful in 2s
CI and images / build-agent (push) Successful in 4s
CI and images / build-web (push) Successful in 1m59s
CI and images / smoke-web (push) Successful in 49s
CI and images / promote (push) Successful in 1s
CI and images / refresh-dev (push) Skipped
CI and images / unattended-failure (push) Skipped
Importer._import_media and Importer.attach_in_place each carried their own copy of the still-image filters (verify, minimum size, transparency, single colour) and of the phash step, and the copies had drifted: attach_in_place's transparency check had no guard for an OSError from decoding truncated pixel data, so that error escaped into DownloadService's per-file loop and ended the run with the remaining files unattached; its phash step swallowed every error and imported the broken file without a hash. Both paths now call _screen_still and _phash_of, which turn a decode error into an invalid_image skip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -869,7 +869,6 @@ class Importer:
|
||||
# Compute file dimensions (images only) and apply filters.
|
||||
width = height = None
|
||||
duration = None # video container duration (Tier-1 near-dup key, #871)
|
||||
has_alpha = False
|
||||
if is_video(source):
|
||||
# Layer-3 isolation: validate the container via ffprobe (a
|
||||
# separate process) before the rest of the pipeline touches
|
||||
@@ -888,52 +887,9 @@ class Importer:
|
||||
)
|
||||
width, height, duration = probe.width, probe.height, probe.duration
|
||||
else:
|
||||
try:
|
||||
with Image.open(source) as im:
|
||||
im.verify()
|
||||
with Image.open(source) as im:
|
||||
width, height = im.size
|
||||
has_alpha = im.mode in ("RGBA", "LA") or (
|
||||
im.mode == "P" and "transparency" in im.info
|
||||
)
|
||||
except Exception as exc:
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.invalid_image, error=str(exc),
|
||||
)
|
||||
|
||||
if (self.settings.min_width and width < self.settings.min_width) or (
|
||||
self.settings.min_height and height < self.settings.min_height
|
||||
):
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.too_small,
|
||||
error=f"{width}x{height} below min {self.settings.min_width}x{self.settings.min_height}",
|
||||
)
|
||||
|
||||
if self.settings.skip_transparent and has_alpha:
|
||||
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:
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.too_transparent,
|
||||
error=f"{pct:.2%} transparent",
|
||||
)
|
||||
|
||||
if self.settings.skip_single_color and self._single_color_hit(source):
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.single_color,
|
||||
error=(f"one color dominates >"
|
||||
f"{self.settings.single_color_threshold:.0%}"),
|
||||
)
|
||||
refusal, width, height = self._screen_still(source)
|
||||
if refusal is not None:
|
||||
return refusal
|
||||
|
||||
# Artist anchored to the attribution path (folder→artist), resolved
|
||||
# UP-FRONT so the enrich-on-duplicate branches link provenance with the
|
||||
@@ -958,16 +914,9 @@ class Importer:
|
||||
# Perceptual near-dup (images only; videos keep phash NULL).
|
||||
phash = None
|
||||
if not is_video(source):
|
||||
try:
|
||||
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}",
|
||||
)
|
||||
refusal, phash = self._phash_of(source)
|
||||
if refusal is not None:
|
||||
return refusal
|
||||
if phash is not None:
|
||||
candidates = self._phash_candidates_cache()
|
||||
rel, match_id = find_similar(
|
||||
@@ -1309,47 +1258,13 @@ class Importer:
|
||||
path, post=post, artist=artist, resolved=True,
|
||||
)
|
||||
|
||||
# Format / dimension / transparency filters (mirror _import_media).
|
||||
# The same content filters as _import_media.
|
||||
width = height = None
|
||||
duration = None # video container duration (Tier-1 near-dup key, #871)
|
||||
has_alpha = False
|
||||
if not is_video(path):
|
||||
try:
|
||||
with Image.open(path) as im:
|
||||
im.verify()
|
||||
with Image.open(path) as im:
|
||||
width, height = im.size
|
||||
has_alpha = im.mode in ("RGBA", "LA") or (
|
||||
im.mode == "P" and "transparency" in im.info
|
||||
)
|
||||
except Exception as exc:
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.invalid_image,
|
||||
error=str(exc),
|
||||
)
|
||||
|
||||
if (self.settings.min_width and width < self.settings.min_width) or (
|
||||
self.settings.min_height and height < self.settings.min_height
|
||||
):
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.too_small,
|
||||
error=f"{width}x{height} below min {self.settings.min_width}x{self.settings.min_height}",
|
||||
)
|
||||
|
||||
if self.settings.skip_transparent and has_alpha:
|
||||
pct = self._transparency_pct(path)
|
||||
if pct >= self.settings.transparency_threshold:
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.too_transparent,
|
||||
error=f"{pct:.2%} transparent",
|
||||
)
|
||||
|
||||
if self.settings.skip_single_color and self._single_color_hit(path):
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.single_color,
|
||||
error=(f"one color dominates >"
|
||||
f"{self.settings.single_color_threshold:.0%}"),
|
||||
)
|
||||
refusal, width, height = self._screen_still(path)
|
||||
if refusal is not None:
|
||||
return refusal
|
||||
else:
|
||||
# Best-effort probe for dims + duration so downloaded videos can dedup
|
||||
# (#871). LENIENT: unlike _import_media this path does not reject on a
|
||||
@@ -1377,11 +1292,9 @@ class Importer:
|
||||
# phash dedup + supersede
|
||||
phash = None
|
||||
if not is_video(path):
|
||||
try:
|
||||
with Image.open(path) as im:
|
||||
phash = compute_phash(im)
|
||||
except Exception:
|
||||
phash = None
|
||||
refusal, phash = self._phash_of(path)
|
||||
if refusal is not None:
|
||||
return refusal
|
||||
if phash is not None:
|
||||
candidates = self._phash_candidates_cache()
|
||||
rel, match_id = find_similar(
|
||||
@@ -1890,6 +1803,81 @@ class Importer:
|
||||
# is operator-tunable per surface.
|
||||
_SINGLE_COLOR_TOLERANCE = 30
|
||||
|
||||
def _screen_still(
|
||||
self, source: Path,
|
||||
) -> tuple[ImportResult | None, int | None, int | None]:
|
||||
"""Read a still image's size and apply the library's content filters.
|
||||
|
||||
Returns `(refusal, width, height)`. `refusal` is a skip result when
|
||||
the file is not a readable image, is below the minimum size, is mostly
|
||||
transparent or is one colour, and None when the file passes.
|
||||
|
||||
A file whose header verifies but whose pixel data is truncated only
|
||||
fails once PIL decodes it, as the transparency check does. That decode
|
||||
error is a skip too: raised, it would retry the import task forever,
|
||||
or abort a download run in the middle of its files.
|
||||
"""
|
||||
try:
|
||||
with Image.open(source) as im:
|
||||
im.verify()
|
||||
with Image.open(source) as im:
|
||||
width, height = im.size
|
||||
has_alpha = im.mode in ("RGBA", "LA") or (
|
||||
im.mode == "P" and "transparency" in im.info
|
||||
)
|
||||
except Exception as exc:
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.invalid_image,
|
||||
error=str(exc),
|
||||
), None, None
|
||||
|
||||
if (self.settings.min_width and width < self.settings.min_width) or (
|
||||
self.settings.min_height and height < self.settings.min_height
|
||||
):
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.too_small,
|
||||
error=f"{width}x{height} below min {self.settings.min_width}x{self.settings.min_height}",
|
||||
), width, height
|
||||
|
||||
if self.settings.skip_transparent and has_alpha:
|
||||
try:
|
||||
pct = self._transparency_pct(source)
|
||||
except OSError as exc:
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.invalid_image,
|
||||
error=f"PIL load failed during transparency check: {exc}",
|
||||
), width, height
|
||||
if pct >= self.settings.transparency_threshold:
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.too_transparent,
|
||||
error=f"{pct:.2%} transparent",
|
||||
), width, height
|
||||
|
||||
if self.settings.skip_single_color and self._single_color_hit(source):
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.single_color,
|
||||
error=(f"one color dominates >"
|
||||
f"{self.settings.single_color_threshold:.0%}"),
|
||||
), width, height
|
||||
|
||||
return None, width, height
|
||||
|
||||
def _phash_of(self, source: Path) -> tuple[ImportResult | None, object]:
|
||||
"""The perceptual hash of a still image, as `(refusal, phash)`.
|
||||
|
||||
Hashing decodes every pixel, so this is where a file with truncated
|
||||
pixel data can first fail; that is a skip rather than an exception,
|
||||
for the same reason as in `_screen_still`.
|
||||
"""
|
||||
try:
|
||||
with Image.open(source) as im:
|
||||
return None, compute_phash(im)
|
||||
except OSError as exc:
|
||||
return ImportResult(
|
||||
status="skipped", skip_reason=SkipReason.invalid_image,
|
||||
error=f"PIL load failed during phash compute: {exc}",
|
||||
), None
|
||||
|
||||
def _single_color_hit(self, source: Path) -> bool:
|
||||
"""True when one color dominates beyond the configured threshold — the
|
||||
same canonical predicate the Cleanup audit runs (audits.single_color,
|
||||
|
||||
Reference in New Issue
Block a user