fix: a game build is recognised before the archive integrity test, so re-extract no longer stalls on it
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 18s
CI and images / backend-lint-and-test (push) Successful in 47s
CI and images / integration (push) Successful in 2m44s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-agent (push) Successful in 5s
CI and images / build-web (push) Successful in 2m1s
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 18s
CI and images / backend-lint-and-test (push) Successful in 47s
CI and images / integration (push) Successful in 2m44s
CI and images / sign-extension (push) Successful in 3s
CI and images / build-agent (push) Successful in 5s
CI and images / build-web (push) Successful in 2m1s
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
The archive probe read and checksummed every member before the importer looked at what the archive held. For a multi-gigabyte game build that is minutes of work on a file that is kept whole and never extracted, and it held a re-extract chunk far past its time budget. - The importer checks the archive's directory for a software build first, for the outer archive and for one nested inside an art pack, and only probes archives it may extract. - The probe rejects an archive over the size cap from its directory totals, before the integrity test decompresses anything. - Re-extract logs each archive as it starts and finishes, with its size and how long it took, so an archive that stalls a chunk is named in the log. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1205,6 +1205,11 @@ def reextract_archive_attachments(
|
|||||||
)
|
)
|
||||||
if post is None:
|
if post is None:
|
||||||
summary["without_post"] += 1
|
summary["without_post"] += 1
|
||||||
|
# Logged on the way in as well as out, so an archive that stalls a
|
||||||
|
# chunk is named in the log even if the worker dies holding it.
|
||||||
|
size_mb = (att.size_bytes or 0) / 1e6
|
||||||
|
log.info("re-extract: attachment %s (%.0f MB) starting", att.id, size_mb)
|
||||||
|
archive_start = time.monotonic()
|
||||||
try:
|
try:
|
||||||
ids = _reextract_archive_to_post(
|
ids = _reextract_archive_to_post(
|
||||||
importer, stored, post, source_row, artist, images_root,
|
importer, stored, post, source_row, artist, images_root,
|
||||||
@@ -1214,8 +1219,11 @@ def reextract_archive_attachments(
|
|||||||
except Exception as exc: # one bad archive must not strand the rest
|
except Exception as exc: # one bad archive must not strand the rest
|
||||||
session.rollback()
|
session.rollback()
|
||||||
summary["errors"] += 1
|
summary["errors"] += 1
|
||||||
log.warning("re-extract failed for attachment %s: %s", att.id, exc)
|
log.warning("re-extract failed for attachment %s after %.1fs: %s",
|
||||||
|
att.id, time.monotonic() - archive_start, exc)
|
||||||
continue
|
continue
|
||||||
|
log.info("re-extract: attachment %s done in %.1fs, %d new image(s)",
|
||||||
|
att.id, time.monotonic() - archive_start, len(ids))
|
||||||
if ids:
|
if ids:
|
||||||
summary["members_imported"] += len(ids)
|
summary["members_imported"] += len(ids)
|
||||||
if post is not None:
|
if post is not None:
|
||||||
|
|||||||
@@ -594,29 +594,22 @@ class Importer:
|
|||||||
# lays files out under /images/<artist_slug>/...). Filesystem
|
# lays files out under /images/<artist_slug>/...). Filesystem
|
||||||
# import still calls bare _import_archive(source) and falls
|
# import still calls bare _import_archive(source) and falls
|
||||||
# back to the path-walk derivation as before.
|
# back to the path-walk derivation as before.
|
||||||
probe = safe_probe.probe_archive(source)
|
#
|
||||||
if not probe.ok:
|
# A game or app build is recognised first, from the archive's directory
|
||||||
if probe.crashed:
|
# alone. The probe reads and checksums every member, which for a
|
||||||
|
# multi-gigabyte build is minutes of work on a file that is kept whole
|
||||||
|
# and never extracted anyway.
|
||||||
|
marker = software_marker(source)
|
||||||
|
probe = None if marker is not None else safe_probe.probe_archive(source)
|
||||||
|
if probe is not None and probe.crashed:
|
||||||
return ImportResult(
|
return ImportResult(
|
||||||
status="failed",
|
status="failed",
|
||||||
error=f"archive probe crashed/timed out: {probe.reason}",
|
error=f"archive probe crashed/timed out: {probe.reason}",
|
||||||
)
|
)
|
||||||
# Clean rejection (bomb cap exceeded, integrity mismatch):
|
|
||||||
# still preserve the archive file itself as an attachment so
|
|
||||||
# nothing silently vanishes, matching extract_archive's
|
|
||||||
# fail-soft contract.
|
|
||||||
artist_use = artist if artist is not None else self._resolve_artist(source)
|
|
||||||
post = self._post_for_sidecar(source, artist_use, source_row=source_row)
|
|
||||||
self._capture_attachment(
|
|
||||||
source, post=post, artist=artist_use, resolved=True,
|
|
||||||
)
|
|
||||||
reason = f"archive probe rejected, captured unextracted: {probe.reason}"
|
|
||||||
log.warning("%s: %s", source.name, reason)
|
|
||||||
return ImportResult(status="attached", error=reason)
|
|
||||||
|
|
||||||
artist_use = artist if artist is not None else self._resolve_artist(source)
|
artist_use = artist if artist is not None else self._resolve_artist(source)
|
||||||
post = self._post_for_sidecar(source, artist_use, source_row=source_row)
|
post = self._post_for_sidecar(source, artist_use, source_row=source_row)
|
||||||
if (marker := software_marker(source)) is not None:
|
if marker is not None:
|
||||||
# A game or app build: kept whole, its bundled images left alone.
|
# A game or app build: kept whole, its bundled images left alone.
|
||||||
self._capture_attachment(
|
self._capture_attachment(
|
||||||
source, post=post, artist=artist_use, resolved=True,
|
source, post=post, artist=artist_use, resolved=True,
|
||||||
@@ -624,6 +617,16 @@ class Importer:
|
|||||||
log.info("%s: a software build (holds %s), kept unextracted",
|
log.info("%s: a software build (holds %s), kept unextracted",
|
||||||
source.name, marker)
|
source.name, marker)
|
||||||
return ImportResult(status="attached")
|
return ImportResult(status="attached")
|
||||||
|
if not probe.ok:
|
||||||
|
# Clean rejection (bomb cap exceeded, integrity mismatch): the
|
||||||
|
# archive file itself is still kept as an attachment so nothing
|
||||||
|
# silently vanishes, matching extract_archive's fail-soft contract.
|
||||||
|
self._capture_attachment(
|
||||||
|
source, post=post, artist=artist_use, resolved=True,
|
||||||
|
)
|
||||||
|
reason = f"archive probe rejected, captured unextracted: {probe.reason}"
|
||||||
|
log.warning("%s: %s", source.name, reason)
|
||||||
|
return ImportResult(status="attached", error=reason)
|
||||||
# Committed before extraction, so a member that fails (and is rolled
|
# Committed before extraction, so a member that fails (and is rolled
|
||||||
# back on its own) can never take the archive's post with it.
|
# back on its own) can never take the archive's post with it.
|
||||||
self.session.commit()
|
self.session.commit()
|
||||||
@@ -718,6 +721,15 @@ class Importer:
|
|||||||
_ARCHIVE_MAX_DEPTH, member_path.name,
|
_ARCHIVE_MAX_DEPTH, member_path.name,
|
||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
|
# Recognised from its directory before the probe reads
|
||||||
|
# every member, as for the outer archive.
|
||||||
|
if (marker := software_marker(member_path)) is not None:
|
||||||
|
counts["nested_software"] += 1
|
||||||
|
log.info(
|
||||||
|
"nested archive is a software build (holds %s), "
|
||||||
|
"skipped: %s", marker, member_path.name,
|
||||||
|
)
|
||||||
|
continue
|
||||||
probe = safe_probe.probe_archive(member_path)
|
probe = safe_probe.probe_archive(member_path)
|
||||||
if not probe.ok:
|
if not probe.ok:
|
||||||
counts["nested_rejected"] += 1
|
counts["nested_rejected"] += 1
|
||||||
@@ -726,13 +738,6 @@ class Importer:
|
|||||||
probe.reason, member_path.name,
|
probe.reason, member_path.name,
|
||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
if (marker := software_marker(member_path)) is not None:
|
|
||||||
counts["nested_software"] += 1
|
|
||||||
log.info(
|
|
||||||
"nested archive is a software build (holds %s), "
|
|
||||||
"skipped: %s", marker, member_path.name,
|
|
||||||
)
|
|
||||||
continue
|
|
||||||
self._collect_archive_members(
|
self._collect_archive_members(
|
||||||
member_path, attribution=attribution,
|
member_path, attribution=attribution,
|
||||||
source_row=source_row, depth=depth + 1,
|
source_row=source_row, depth=depth + 1,
|
||||||
|
|||||||
@@ -175,12 +175,20 @@ def _run_probe(path_str: str) -> tuple[str, str | None]:
|
|||||||
return ("ok", None)
|
return ("ok", None)
|
||||||
|
|
||||||
|
|
||||||
|
def _over_cap(total) -> bool:
|
||||||
|
return total is not None and total > MAX_ARCHIVE_UNCOMPRESSED_BYTES
|
||||||
|
|
||||||
|
|
||||||
def _inspect_archive(path: Path):
|
def _inspect_archive(path: Path):
|
||||||
"""Return (total_uncompressed_bytes | None, first_bad_member | None)
|
"""Return (total_uncompressed_bytes | None, first_bad_member | None)
|
||||||
for the archive. Format detected by extension OR magic bytes (so a
|
for the archive. Format detected by extension OR magic bytes (so a
|
||||||
mis-named archive is still bomb-guarded + integrity-tested, matching the
|
mis-named archive is still bomb-guarded + integrity-tested, matching the
|
||||||
extractor's gate); raises on a structurally-broken container (caught by the
|
extractor's gate); raises on a structurally-broken container (caught by the
|
||||||
child as a clean rejection)."""
|
child as a clean rejection).
|
||||||
|
|
||||||
|
The size total comes from the archive's directory, so an archive over the
|
||||||
|
cap is rejected before the integrity test decompresses anything: on a
|
||||||
|
multi-gigabyte archive that test alone outlasts the probe's timeout."""
|
||||||
from ..services.archive_extractor import detect_archive_format
|
from ..services.archive_extractor import detect_archive_format
|
||||||
|
|
||||||
fmt = detect_archive_format(path)
|
fmt = detect_archive_format(path)
|
||||||
@@ -189,12 +197,16 @@ def _inspect_archive(path: Path):
|
|||||||
|
|
||||||
with zipfile.ZipFile(path) as zf:
|
with zipfile.ZipFile(path) as zf:
|
||||||
total = sum(zi.file_size for zi in zf.infolist())
|
total = sum(zi.file_size for zi in zf.infolist())
|
||||||
|
if _over_cap(total):
|
||||||
|
return total, None
|
||||||
return total, zf.testzip()
|
return total, zf.testzip()
|
||||||
if fmt == "rar":
|
if fmt == "rar":
|
||||||
import rarfile
|
import rarfile
|
||||||
|
|
||||||
with rarfile.RarFile(path) as rf:
|
with rarfile.RarFile(path) as rf:
|
||||||
total = sum(getattr(ri, "file_size", 0) for ri in rf.infolist())
|
total = sum(getattr(ri, "file_size", 0) for ri in rf.infolist())
|
||||||
|
if _over_cap(total):
|
||||||
|
return total, None
|
||||||
rf.testrar()
|
rf.testrar()
|
||||||
return total, None
|
return total, None
|
||||||
if fmt == "7z":
|
if fmt == "7z":
|
||||||
@@ -203,6 +215,8 @@ def _inspect_archive(path: Path):
|
|||||||
with py7zr.SevenZipFile(path, "r") as zf:
|
with py7zr.SevenZipFile(path, "r") as zf:
|
||||||
info = zf.archiveinfo()
|
info = zf.archiveinfo()
|
||||||
total = getattr(info, "uncompressed", None)
|
total = getattr(info, "uncompressed", None)
|
||||||
|
if _over_cap(total):
|
||||||
|
return total, None
|
||||||
ok = zf.test() # True / None when all members pass
|
ok = zf.test() # True / None when all members pass
|
||||||
return total, (None if ok in (True, None) else "7z test reported corruption")
|
return total, (None if ok in (True, None) else "7z test reported corruption")
|
||||||
# Not a recognised archive — nothing to test; treat as clean.
|
# Not a recognised archive — nothing to test; treat as clean.
|
||||||
|
|||||||
@@ -305,3 +305,24 @@ def test_a_game_build_inside_an_art_pack_is_skipped_and_the_art_imported(importe
|
|||||||
assert r.status == "imported"
|
assert r.status == "imported"
|
||||||
assert len(r.member_image_ids) == 1
|
assert len(r.member_image_ids) == 1
|
||||||
assert importer.session.execute(select(func.count()).select_from(ImageRecord)).scalar_one() == 1
|
assert importer.session.execute(select(func.count()).select_from(ImageRecord)).scalar_one() == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_game_build_is_recognised_before_the_integrity_test(
|
||||||
|
importer, import_layout, monkeypatch,
|
||||||
|
):
|
||||||
|
"""A multi-gigabyte build would spend minutes in the probe's read-everything
|
||||||
|
test, only to be kept whole anyway, so its directory decides first."""
|
||||||
|
from backend.app.utils import safe_probe
|
||||||
|
|
||||||
|
def _no_probe(_path, **_kw):
|
||||||
|
raise AssertionError("a software build went through the archive probe")
|
||||||
|
monkeypatch.setattr(safe_probe, "probe_archive", _no_probe)
|
||||||
|
import_root, _ = import_layout
|
||||||
|
arc = import_root / "Bob" / "MyGame-2.0-pc.zip"
|
||||||
|
arc.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
with zipfile.ZipFile(arc, "w") as zf:
|
||||||
|
zf.writestr("MyGame/MyGame.exe", b"MZ")
|
||||||
|
zf.writestr("MyGame/game/images/cg01.jpg", _split_bytes("v"))
|
||||||
|
|
||||||
|
assert importer.import_one(arc).status == "attached"
|
||||||
|
assert importer.session.execute(select(func.count()).select_from(ImageRecord)).scalar_one() == 0
|
||||||
|
|||||||
@@ -67,6 +67,23 @@ def test_run_probe_bomb_guard(tmp_path, monkeypatch):
|
|||||||
assert "bomb-guard cap" in detail
|
assert "bomb-guard cap" in detail
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_archive_over_the_cap_is_rejected_without_an_integrity_test(
|
||||||
|
tmp_path, monkeypatch,
|
||||||
|
):
|
||||||
|
"""The size total is read from the directory, so an oversize archive never
|
||||||
|
reaches the decompress-everything test."""
|
||||||
|
monkeypatch.setattr(safe_probe, "MAX_ARCHIVE_UNCOMPRESSED_BYTES", 10)
|
||||||
|
|
||||||
|
def _no_test(_self):
|
||||||
|
raise AssertionError("integrity test ran on an archive over the cap")
|
||||||
|
monkeypatch.setattr(zipfile.ZipFile, "testzip", _no_test)
|
||||||
|
z = tmp_path / "big.zip"
|
||||||
|
_zip(z, {"big.txt": b"x" * 5000})
|
||||||
|
status, detail = safe_probe._run_probe(str(z))
|
||||||
|
assert status == "error"
|
||||||
|
assert "bomb-guard cap" in detail
|
||||||
|
|
||||||
|
|
||||||
def test_probe_video_non_video_is_not_ok(tmp_path):
|
def test_probe_video_non_video_is_not_ok(tmp_path):
|
||||||
"""A text file is not a decodable video. Whether ffprobe is present
|
"""A text file is not a decodable video. Whether ffprobe is present
|
||||||
(returncode != 0) or absent (OSError → 'unavailable'), the result is
|
(returncode != 0) or absent (OSError → 'unavailable'), the result is
|
||||||
|
|||||||
Reference in New Issue
Block a user