diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index 24d73bd..7363125 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -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, diff --git a/tests/test_importer.py b/tests/test_importer.py index f8b7af8..c5fd7b2 100644 --- a/tests/test_importer.py +++ b/tests/test_importer.py @@ -216,6 +216,28 @@ def test_pil_load_oserror_in_transparency_check_skips_not_raises( assert "transparency check" in (result.error or "") +def test_downloaded_file_with_broken_pixels_skips_not_raises( + importer, import_layout, monkeypatch, +): + """A downloaded file goes through the same filters as an imported one, so + a decode error there is a skip; raised, it would end the download run + before the rest of its files were attached.""" + _, images_root = import_layout + path = images_root / "Bob" / "corrupt.png" + _make_png_rgba(path, (100, 100), alpha=128) + importer.settings.skip_transparent = True + importer.settings.transparency_threshold = 0.5 + + def _boom(_self, _src): + raise OSError("broken data stream when reading image file") + monkeypatch.setattr(type(importer), "_transparency_pct", _boom) + + result = importer.attach_in_place(path) + 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, ):