diff --git a/booth/thumbs.py b/booth/thumbs.py index c37a426..bbd6bba 100644 --- a/booth/thumbs.py +++ b/booth/thumbs.py @@ -12,8 +12,12 @@ A tile renders a few hundred px wide, so the gallery shipped roughly 16x the pix that reach the screen and 77 MB on one page load. 66 is a fine count sitting on a terrible payload; the operator found it in about a minute of using the Desk. -The cache lives at `/.thumbs/.webp` — inside the booth on purpose, -so it is swept with the booth and never outlives what it describes. Both +The cache lives at `/.thumbs/..webp` (see `thumb_path`), +inside the booth on purpose, so it is swept with the booth and never outlives +what it describes. And because it is inside the booth, where any fleet session +can write, every entry on the way to it may be planted: the cache directories, +the cache file and the temp file are each checked or created so that a link, +a directory or a planted file cannot redirect a write or pose as a thumbnail. Both `booth_items` and `zip_booth` skip every dot-prefixed path COMPONENT, which they did not do until this module needed them to. """ @@ -21,12 +25,16 @@ did not do until this module needed them to. from __future__ import annotations import os +import stat +import tempfile from pathlib import Path try: # optional: absence degrades to full-size images, never to a broken page from PIL import Image as _Image + from PIL import ImageOps as _ImageOps except ImportError: # pragma: no cover _Image = None + _ImageOps = None THUMB_DIR = ".thumbs" # SIZED FOR THE TILE'S WIDTH, AT 2x DENSITY. A gallery tile is sized by its @@ -48,6 +56,14 @@ THUMB_HEIGHT_MAX = 4096 # thumbnails average 39 KB, so anything at or under 64 KB has nothing to save. THUMB_LIGHT_BYTES = 64 * 1024 THUMB_QUALITY = 78 +# A header is free to read and claims any size it likes; `thumbnail()` then +# decodes it, on every request, because a failure is not cached. Past this the +# original is served and nothing is decoded. 64 MP is 8K x 8K, far past any +# image a booth has held. +THUMB_MAX_PIXELS = 64_000_000 +# Bump when the ENCODING changes in a way the numbers above do not show (a mode +# conversion, an orientation rule), so every cached thumbnail is rebuilt. +THUMB_VERSION = 2 # What Pillow can open from a plain install. SVG is vector (Pillow cannot read # it, and it is already small); AVIF needs a plugin we do not require. @@ -58,11 +74,13 @@ def thumb_path(booth: Path, rel: str) -> Path: """Where `rel`'s thumbnail lives. Mirrors the tree so two files with the same basename in different folders cannot collide. - The SIZE RULE IS IN THE NAME. The mtime check below only notices a changed - source, so a thumbnail cut to an older rule, newer than its source, would be - served forever. Naming the width means a change to the rule is a cache miss, - and the old files are orphans swept with their booth.""" - return booth / THUMB_DIR / f"{rel}.{THUMB_WIDTH}w.webp" + THE WHOLE RULE IS IN THE NAME: width, height cap, quality and an encoding + version. The freshness check below only notices a changed source, so a + thumbnail cut to an older rule would otherwise be served forever. A change + to any of them is a cache miss, and the old files are orphans swept with + their booth.""" + rule = f"{THUMB_WIDTH}x{THUMB_HEIGHT_MAX}q{THUMB_QUALITY}v{THUMB_VERSION}" + return booth / THUMB_DIR / f"{rel}.{rule}.webp" def wants_thumb(rel: str) -> bool: @@ -73,18 +91,66 @@ def wants_thumb(rel: str) -> bool: return Path(rel).suffix.lower() in THUMBABLE +def _fresh(out: Path, s_stat: os.stat_result) -> bool: + """A cache hit: a REGULAR file (lstat, so a link or a directory planted at + the name never counts) carrying its source's EXACT mtime. Exact, not + "at least as new": a source replaced by `cp -p` or an archive extract keeps + an OLDER stamp, and `>=` served the old thumbnail forever.""" + try: + o = os.lstat(out) + except OSError: + return False + return stat.S_ISREG(o.st_mode) and o.st_mtime_ns == s_stat.st_mtime_ns + + +def _cache_dir(booth: Path, parent: Path) -> bool: + """Make `parent` (a directory under `booth`) exist as REAL directories, + component by component, never following a link. False when something that + is not a directory is in the way: a `.thumbs` planted as a link would + otherwise put the cache outside the booth, beyond the sweep. + + ⚠ CREATING `.thumbs` TOUCHES THE BOOTH DIRECTORY'S OWN MTIME, and + `_newest_mtime` seeds from exactly that — so merely LOOKING at a booth aged + it, and once the Desk pulls a thumbnail per booth, one index load would push + every expiry out and the TTL would never fire again. Excluding the cache's + CONTENTS is not enough; the directory entry is the leak. So the booth's + mtime is put back after `.thumbs` is made. That cannot hide real activity: + any file an agent adds is counted by its OWN mtime in the same walk, and the + directory stamp is only the seed.""" + cur = booth + for part in parent.relative_to(booth).parts: + cur = cur / part + try: + if not stat.S_ISDIR(os.lstat(cur).st_mode): + return False + continue + except FileNotFoundError: + pass + restore = booth.stat() if cur.parent == booth else None + try: + os.mkdir(cur) + except FileExistsError: + pass + if restore is not None: + try: + os.utime(booth, ns=(restore.st_atime_ns, restore.st_mtime_ns)) + except OSError: + pass + if not stat.S_ISDIR(os.lstat(cur).st_mode): + return False + return True + + def ensure_thumb(booth: Path, rel: str) -> Path | None: """The cached thumbnail for `rel`, generating it if needed. None when there - should not be one — Pillow absent, unsupported type, source already tile-sized - and light (or animated, since a thumbnail is one frame), or anything at all - went wrong. + should not be one — Pillow absent, unsupported type, source already + tile-sized and light, an animation that already fits (a thumbnail is one + frame; an animation too big to fit IS flattened), over the pixel budget, + something planted in the cache's way, or anything at all went wrong. NEVER RAISES. A thumbnail is an optimisation; a booth page that will not load is worse than a page that loads slowly, which is the posture every other read on this path already takes. - - Regenerates when the source is newer than the cache, so editing a file in - place does not leave the old thumbnail behind forever. """ if _Image is None or not wants_thumb(rel): return None @@ -92,49 +158,49 @@ def ensure_thumb(booth: Path, rel: str) -> Path | None: out = thumb_path(booth, rel) try: s_stat = src.stat() - try: - if out.stat().st_mtime >= s_stat.st_mtime: - return out - except OSError: - pass # no cache yet, or unreadable — fall through and build one + if _fresh(out, s_stat): + return out with _Image.open(src) as im: # `open` reads the header only, so this is cheap enough to decide on. - fits = im.width <= THUMB_WIDTH and im.height <= THUMB_HEIGHT_MAX + w, h = im.size + if w * h > THUMB_MAX_PIXELS: + return None + # The size the picture is SEEN at: a camera stores a portrait + # sideways and says so in EXIF, and the browser honours it on the + # original. Pillow does not, so sizing the raw pixels tiled a + # portrait as a landscape (groa, seat-verified). + orientation = im.getexif().get(0x0112, 1) + if orientation in (5, 6, 7, 8): + w, h = h, w + fits = w <= THUMB_WIDTH and h <= THUMB_HEIGHT_MAX if fits and (s_stat.st_size <= THUMB_LIGHT_BYTES or getattr(im, "is_animated", False)): return None # already tile-sized and cheap (or moving): serve the original + if orientation != 1: + im = _ImageOps.exif_transpose(im) im.thumbnail((THUMB_WIDTH, THUMB_HEIGHT_MAX)) if im.mode not in ("RGB", "RGBA"): - im = im.convert("RGBA" if "A" in im.getbands() else "RGB") - # ⚠ CREATING THE CACHE DIR TOUCHES THE BOOTH DIRECTORY'S OWN - # MTIME, and `_newest_mtime` seeds from exactly that — so merely - # LOOKING at a booth aged it, and once the Desk pulls a thumbnail - # per booth, one index load would push every expiry out and the TTL - # would never fire again. Excluding the cache's CONTENTS is not - # enough; the directory entry is the leak. - # - # Restoring the booth's mtime cannot hide real activity: any file an - # agent adds is counted by its OWN mtime in the same walk, and the - # directory stamp is only the seed. - made = not out.parent.exists() - booth_stat = booth.stat() if made else None - out.parent.mkdir(parents=True, exist_ok=True) - if booth_stat is not None: - try: - os.utime(booth, ns=(booth_stat.st_atime_ns, booth_stat.st_mtime_ns)) - except OSError: - pass - # Atomic, like every other sidecar this service writes: a reader - # polling the cache never sees a half-encoded image. - tmp = out.with_name(out.name + f".{os.getpid()}.tmp") + # A palette PNG carries transparency in `info`, not as a band: + # `getbands()` alone baked it opaque (3/4 arms, seat-executed). + alpha = "A" in im.getbands() or "transparency" in im.info + im = im.convert("RGBA" if alpha else "RGB") + if not _cache_dir(booth, out.parent): + return None + # Atomic, like every other sidecar this service writes, through a + # temp file created O_EXCL under an unpredictable name: the old + # `..tmp` could be planted as a link, and the encoder + # wrote THROUGH it (seat P5: 600 B -> 316,400 B). + fd, tmp = tempfile.mkstemp(prefix=".", suffix=".tmp", dir=out.parent) try: - im.save(tmp, "WEBP", quality=THUMB_QUALITY, method=4) + with os.fdopen(fd, "wb") as fh: + im.save(fh, "WEBP", quality=THUMB_QUALITY, method=4) + os.utime(tmp, ns=(s_stat.st_atime_ns, s_stat.st_mtime_ns)) os.replace(tmp, out) finally: try: - tmp.unlink() + os.unlink(tmp) except OSError: pass - return out + return out if _fresh(out, s_stat) else None except Exception: # noqa: BLE001 — a bad image costs its own tile, never the page return None diff --git a/persistent-memory.md b/persistent-memory.md index 64cff79..fc01b3b 100644 --- a/persistent-memory.md +++ b/persistent-memory.md @@ -54,7 +54,14 @@ _As of 2026-09-23:_ so if a redesign widens the tiles, that test goes red. The 2-column (≤472px) and 1-column (≤650px) reflows are softer than 768 covers at 2x; 1024 would cover 2 columns for 18.5 MB total. Four surfaces (tile, Desk strip, flag tray, - filmstrip); the review stage keeps the original. + filmstrip); the review stage keeps the original. The heid bug-hunt (4/4 + arms) folded: a cache hit must be a regular file carrying its source's EXACT + mtime (a planted directory, or a `cp -p` older source, no longer pins a + thumbnail); the cache dirs are made without following links; the temp file is + mkstemp (the old `..tmp` could be planted as a link and was written + through); palette transparency survives; EXIF orientation is honoured; and + there's a 64 MP decode budget. The cache name carries the whole rule + (`.768x4096q78v2.webp`). → `persistent-memory.d/2026-09-23-the-cache-that-aged-the-thing-it-cached.md` - ✅ **CREATION + UPDATE DATES ARE ON THE RECORD** for all 30 booths (`created_at` via `statx`, `landed_at` already existed). design-dev renders diff --git a/tests/mutations/thumbs.toml b/tests/mutations/thumbs.toml index 761332f..f12acd6 100644 --- a/tests/mutations/thumbs.toml +++ b/tests/mutations/thumbs.toml @@ -64,6 +64,79 @@ label = "an unversioned cache name: a thumbnail cut to the old rule is served fo file = "booth/thumbs.py" test = "tests/test_thumbs.py::test_a_thumbnail_cut_to_the_old_rule_is_not_served" old = ''' - return booth / THUMB_DIR / f"{rel}.{THUMB_WIDTH}w.webp"''' + return booth / THUMB_DIR / f"{rel}.{rule}.webp"''' new = ''' return booth / THUMB_DIR / (rel + ".webp")''' + +# ---- the heid bug-hunt on this change (4/4 arms), folded ------------------------ + +[[mutation]] +label = "a cache hit trusts the name and the mtime (a planted directory is served)" +file = "booth/thumbs.py" +test = "tests/test_thumbs.py::test_a_planted_directory_at_the_cache_path_is_not_served" +old = ''' + return stat.S_ISREG(o.st_mode) and o.st_mtime_ns == s_stat.st_mtime_ns''' +new = ''' + return o.st_mtime_ns >= s_stat.st_mtime_ns''' + +[[mutation]] +label = "freshness is 'at least as new' (a cp -p'd older source pins the old thumbnail)" +file = "booth/thumbs.py" +test = "tests/test_thumbs.py::test_a_source_replaced_with_an_older_mtime_is_rebuilt" +old = ''' + return stat.S_ISREG(o.st_mode) and o.st_mtime_ns == s_stat.st_mtime_ns''' +new = ''' + return stat.S_ISREG(o.st_mode) and o.st_mtime_ns >= s_stat.st_mtime_ns''' + +[[mutation]] +label = "the cache dirs are made by following links (a planted .thumbs link escapes the booth)" +file = "booth/thumbs.py" +test = "tests/test_thumbs.py::test_a_symlinked_cache_dir_is_never_written_through" +old = ''' + if not _cache_dir(booth, out.parent): + return None''' +new = ''' + out.parent.mkdir(parents=True, exist_ok=True)''' + +[[mutation]] +label = "a predictable temp name the encoder writes through" +file = "booth/thumbs.py" +test = "tests/test_thumbs.py::test_a_planted_link_at_the_old_temp_name_cannot_redirect_the_write" +old = ''' + fd, tmp = tempfile.mkstemp(prefix=".", suffix=".tmp", dir=out.parent) + try: + with os.fdopen(fd, "wb") as fh: + im.save(fh, "WEBP", quality=THUMB_QUALITY, method=4)''' +new = ''' + tmp = str(out) + f".{os.getpid()}.tmp" + try: + im.save(tmp, "WEBP", quality=THUMB_QUALITY, method=4)''' + +[[mutation]] +label = "RGBA chosen by getbands() alone (palette transparency baked opaque)" +file = "booth/thumbs.py" +test = "tests/test_thumbs.py::test_palette_transparency_survives_the_thumbnail" +old = ''' + alpha = "A" in im.getbands() or "transparency" in im.info''' +new = ''' + alpha = "A" in im.getbands()''' + +[[mutation]] +label = "EXIF orientation ignored (a camera portrait tiled sideways)" +file = "booth/thumbs.py" +test = "tests/test_thumbs.py::test_a_camera_portrait_is_sized_and_saved_upright" +old = ''' + orientation = im.getexif().get(0x0112, 1)''' +new = ''' + orientation = 1''' + +[[mutation]] +label = "no pixel budget: whatever the header claims is decoded" +file = "booth/thumbs.py" +test = "tests/test_thumbs.py::test_an_image_past_the_pixel_budget_is_never_decoded" +old = ''' + if w * h > THUMB_MAX_PIXELS: + return None''' +new = ''' + if False: + return None''' diff --git a/tests/test_thumbs.py b/tests/test_thumbs.py index 5a4b712..7388464 100644 --- a/tests/test_thumbs.py +++ b/tests/test_thumbs.py @@ -269,3 +269,108 @@ def test_a_thumbnail_cut_to_the_old_rule_is_not_served(tmp_path): out = ensure_thumb(b, "p.png") assert out == thumb_path(b, "p.png") and out != legacy assert PIL.open(out).size == (704, 1408) + + +# ---- the heid bug-hunt on this change (4/4 arms), folded ------------------------ +# +# The cache sits in a directory any fleet session can write into, so every entry +# on the way to it may be planted. The new size rules only governed cache MISSES; +# the hit path trusted a name and an mtime. + + +def test_a_planted_directory_at_the_cache_path_is_not_served(tmp_path): + """4/4, seat-executed: a directory at the cache path, with a future mtime, + was returned AS the thumbnail. Defeating change: a cache hit that checks + only the mtime.""" + import os + b = tmp_path / "g" + _noise(b / "p.png", 704, 1408) + out = thumb_path(b, "p.png") + out.mkdir(parents=True) + os.utime(out, (2e9, 2e9)) + got = ensure_thumb(b, "p.png") + assert got is None or got.is_file() + + +def test_a_source_replaced_with_an_older_mtime_is_rebuilt(tmp_path): + """kimi: `cp -p` or an archive extract keeps an OLDER mtime, and a cache + newer than its source was served forever. The cache now carries its + source's exact mtime, so any change is a miss. Defeating change: `>=`.""" + import os + b = tmp_path / "g" + _noise(b / "p.png", 704, 1408) + first = ensure_thumb(b, "p.png") + assert PIL.open(first).size == (704, 1408) + _noise(b / "p.png", 1536, 768) + os.utime(b / "p.png", (1e9, 1e9)) # an older stamp than the cache + assert PIL.open(ensure_thumb(b, "p.png")).size == (THUMB_WIDTH, THUMB_WIDTH // 2) + + +def test_a_symlinked_cache_dir_is_never_written_through(tmp_path): + """seat P4: `.thumbs` planted as a link to another directory put the cache + outside the booth, beyond the sweep. Defeating change: `mkdir(parents=True)`, + which follows an existing link.""" + b = tmp_path / "g" + _noise(b / "p.png", 704, 1408) + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + (b / THUMB_DIR).symlink_to(elsewhere) + assert ensure_thumb(b, "p.png") is None + assert list(elsewhere.iterdir()) == [] + + +def test_a_planted_link_at_the_old_temp_name_cannot_redirect_the_write(tmp_path): + """groa, seat P5: the temp name was `..tmp`, predictable, so a + link planted there made the encoder truncate and overwrite its target + (600 B -> 316,400 B). Defeating change: any predictable temp name.""" + import os + b = tmp_path / "g" + _noise(b / "p.png", 704, 1408) + victim = tmp_path / "victim.txt" + victim.write_text("untouched") + out = thumb_path(b, "p.png") + out.parent.mkdir(parents=True) + (out.parent / (out.name + f".{os.getpid()}.tmp")).symlink_to(victim) + ensure_thumb(b, "p.png") + assert victim.read_text() == "untouched" + + +def test_palette_transparency_survives_the_thumbnail(tmp_path): + """3/4, seat-executed, and INTRODUCED by this change: the fits-but-heavy + branch newly re-encodes palette PNGs, and `getbands()` of mode P has no A + even with a tRNS chunk, so transparency became opaque. Defeating change: + choosing RGBA by `getbands()` alone.""" + import os + b = tmp_path / "g" + b.mkdir() + im = PIL.frombytes("P", (400, 400), os.urandom(400 * 400)) + im.putpalette(os.urandom(768)) + im.save(b / "p.png", "PNG", transparency=0) + assert (b / "p.png").stat().st_size > THUMB_LIGHT_BYTES + t = PIL.open(ensure_thumb(b, "p.png")) + assert t.mode == "RGBA" and t.getchannel("A").getextrema()[0] == 0 + + +def test_a_camera_portrait_is_sized_and_saved_upright(tmp_path): + """groa, seat-verified: EXIF orientation was ignored, so a portrait shot + stored sideways was sized as a landscape and tiled sideways. Defeating + change: sizing the raw pixels without `exif_transpose`.""" + import os + b = tmp_path / "g" + b.mkdir() + exif = PIL.Exif() + exif[0x0112] = 6 # rotate 90 CW to display + PIL.frombytes("RGB", (1200, 800), os.urandom(1200 * 800 * 3)).save( + b / "cam.jpg", "JPEG", exif=exif, quality=95) + assert PIL.open(ensure_thumb(b, "cam.jpg")).size == (THUMB_WIDTH, 1152) + + +def test_an_image_past_the_pixel_budget_is_never_decoded(tmp_path, monkeypatch): + """2/4: the header is free to read and `thumbnail()` then decodes whatever it + claims, on every request, since a failure is not cached. Over the budget, + the original is served instead. Defeating change: no budget check.""" + import booth.thumbs as thumbs + b = tmp_path / "g" + _noise(b / "p.png", 704, 1408) + monkeypatch.setattr(thumbs, "THUMB_MAX_PIXELS", 704 * 1408 - 1) + assert ensure_thumb(b, "p.png") is None