fix(blur): .blurred round-trips any rel, and one writer serves both surfaces
The heid bug-hunt on r2b merge 1 found the /blur route stripping `f` before
writing, so the form for " a.png" blurred its neighbour "a.png". The route was
only half of it: `.blurred` was one stripped rel per line, so no writer could
store a rel with a leading space or a newline, whatever the route did.
Operator-ruled 2026-09-23 ("fix the blur").
- booth/blur.py (new, stdlib-only): read_blurred / set_blurred / BLUR_FILE.
`.blurred` is now a JSON array in sorted order, the `.seen` shape: opened
O_NOFOLLOW | O_NONBLOCK with an S_ISREG check and a 1 MiB cap, so a planted
symlink is refused and a FIFO can no longer hang every Desk render (the old
read_text() blocked on one). Writes go through mkstemp + os.replace. The
legacy line format is still READ, so the 6 live line-format files keep their
blur until their next write upgrades them. Measured before the change: 42
live rels, none with edge whitespace, so the defect had no live victims.
- The route no longer strips `f`.
- scripts/booth `blur`/`unblur` go through booth.blur.set_blurred instead of
their own grep/printf line writer. Two writers of one format is how the
formats drift, and after this change the shell writer would have appended a
line to a JSON array. Every path is checked before anything is written.
- Item.blurred_self (appended to the record): the item's own blur, resolved in
booth_items from the same read as `blurred`. It replaces build_gallery's
second read_blurred, which a write between the two reads could split
(invariant 3). app.py no longer reads blur state at all, and a test asserts
it.
Names stay importable from booth.app and booth.items (invariant 4). blur joins
test_stdlib_only. test_cli's per-item-survives test now reads through the reader
rather than asserting the old byte format. The r2b contract and its mutation
row follow blurred_self onto the record. tests/mutations/blur_storage.toml
proves 12 falsifiers by running the change each forbids.
Not in this change, and still ours: the "off"-means-ON idiom drift between
/blur, /blurbooth and /flag (forms only ever send 0/1), and the CLI's
`.blurbooth` touch following a symlink where the service no longer does.
This commit is contained in:
@@ -0,0 +1,122 @@
|
||||
# Per-item blur storage: `.blurred` round-trips any rel, whoever writes it.
|
||||
# The fix for the wrong-item write the heid bug-hunt found through r2b merge 1
|
||||
# (a stripped rel blurred its neighbour), operator-ruled 2026-09-23. Every row
|
||||
# is a change tests/test_blur.py claims to forbid.
|
||||
#
|
||||
# NOT here, on purpose: the S_ISREG guard in read_blurred. With O_NONBLOCK a
|
||||
# FIFO opens and reads as EOF, a symlink is already refused by O_NOFOLLOW, and a
|
||||
# device node needs root to plant, so no test here can see that guard go. It
|
||||
# stays as the `.seen` shape, and it is not claimed as a proven falsifier.
|
||||
|
||||
unit = "blur storage round-trip"
|
||||
|
||||
[[mutation]]
|
||||
label = "the writer strips the rel (the old line format's loss)"
|
||||
file = "booth/blur.py"
|
||||
test = "tests/test_blur.py::test_a_leading_space_rel_round_trips"
|
||||
old = '''
|
||||
current.add(rel)'''
|
||||
new = '''
|
||||
current.add(rel.strip())'''
|
||||
|
||||
[[mutation]]
|
||||
label = "the route strips `f` before writing (the reported wrong-item write)"
|
||||
file = "booth/app.py"
|
||||
test = "tests/test_blur.py::test_the_blur_route_blurs_exactly_the_item_it_names"
|
||||
old = '''
|
||||
rel = f.lstrip("/")'''
|
||||
new = '''
|
||||
rel = f.strip().lstrip("/")'''
|
||||
|
||||
[[mutation]]
|
||||
label = "a JSON-only reader: every live line-format file un-blurs on deploy"
|
||||
file = "booth/blur.py"
|
||||
test = "tests/test_blur.py::test_the_legacy_line_format_still_reads"
|
||||
old = '''
|
||||
return {ln.strip() for ln in text.splitlines() if ln.strip()}'''
|
||||
new = '''
|
||||
return set()'''
|
||||
|
||||
[[mutation]]
|
||||
label = "a legacy file that is not JSON reads as nothing instead of falling back"
|
||||
file = "booth/blur.py"
|
||||
test = "tests/test_blur.py::test_a_legacy_rel_that_starts_with_a_bracket_still_reads"
|
||||
old = '''
|
||||
data = None'''
|
||||
new = '''
|
||||
return set()'''
|
||||
|
||||
[[mutation]]
|
||||
label = "a FIFO blocks the read (no O_NONBLOCK)"
|
||||
file = "booth/blur.py"
|
||||
test = "tests/test_blur.py::test_a_fifo_blur_file_does_not_block_the_read"
|
||||
old = '''
|
||||
fd = os.open(booth / BLUR_FILE, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)'''
|
||||
new = '''
|
||||
fd = os.open(booth / BLUR_FILE, os.O_RDONLY | os.O_NOFOLLOW)'''
|
||||
|
||||
[[mutation]]
|
||||
label = "the read follows a planted symlink (no O_NOFOLLOW)"
|
||||
file = "booth/blur.py"
|
||||
test = "tests/test_blur.py::test_a_symlinked_blur_file_is_not_followed_on_read"
|
||||
old = '''
|
||||
fd = os.open(booth / BLUR_FILE, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK)'''
|
||||
new = '''
|
||||
fd = os.open(booth / BLUR_FILE, os.O_RDONLY | os.O_NONBLOCK)'''
|
||||
|
||||
[[mutation]]
|
||||
label = "the write goes through a planted symlink instead of replacing it"
|
||||
file = "booth/blur.py"
|
||||
test = "tests/test_blur.py::test_a_write_replaces_a_planted_symlink_rather_than_writing_through_it"
|
||||
old = '''
|
||||
os.replace(tmp, path)'''
|
||||
new = '''
|
||||
path.write_bytes(Path(tmp).read_bytes()); os.unlink(tmp)'''
|
||||
|
||||
[[mutation]]
|
||||
label = "the stored order is not the stated one (invariant 6)"
|
||||
file = "booth/blur.py"
|
||||
test = "tests/test_blur.py::test_the_file_is_a_json_array_in_sorted_order"
|
||||
old = '''
|
||||
body = json.dumps(sorted(current), ensure_ascii=False)'''
|
||||
new = '''
|
||||
body = json.dumps(sorted(current, reverse=True), ensure_ascii=False)'''
|
||||
|
||||
[[mutation]]
|
||||
label = "the CLI ignores the verb: `unblur` blurs"
|
||||
file = "scripts/booth"
|
||||
test = "tests/test_blur.py::test_the_cli_writes_the_format_the_service_reads"
|
||||
old = '''
|
||||
on = sys.argv[1] == "blur"'''
|
||||
new = '''
|
||||
on = True'''
|
||||
|
||||
[[mutation]]
|
||||
label = "the CLI writes past a refused '..' path"
|
||||
file = "scripts/booth"
|
||||
test = "tests/test_blur.py::test_the_cli_still_refuses_a_dotdot_path"
|
||||
old = '''
|
||||
*..*) echo "refusing path with '..': $item" >&2; exit 2 ;;'''
|
||||
new = '''
|
||||
*..*) echo "refusing path with '..': $item" >&2 ;;'''
|
||||
|
||||
[[mutation]]
|
||||
label = "the item's own blur is the composed one (booth fog leaks into it)"
|
||||
file = "booth/items.py"
|
||||
test = "tests/test_blur.py::test_the_item_record_carries_its_own_blur_apart_from_the_booths"
|
||||
old = '''
|
||||
blurred_self=rel in blurred,'''
|
||||
new = '''
|
||||
blurred_self=rel in blurred or booth_blur,'''
|
||||
|
||||
[[mutation]]
|
||||
label = "app.py reads the blur file a second time (invariant 3)"
|
||||
file = "booth/app.py"
|
||||
test = "tests/test_blur.py::test_app_py_never_reads_the_blur_file_itself"
|
||||
old = '''
|
||||
out = []
|
||||
for it in booth_items(child):'''
|
||||
new = '''
|
||||
out = []
|
||||
read_blurred(child)
|
||||
for it in booth_items(child):'''
|
||||
@@ -134,7 +134,7 @@ label = "D2b the per-item control reads the composed blur, not the item's own"
|
||||
file = "booth/app.py"
|
||||
test = "tests/test_flow.py::test_under_a_fogged_booth_each_items_blur_control_tells_the_truth"
|
||||
old = '''
|
||||
"blurred_self": it.rel in own_blur,'''
|
||||
"blurred_self": it.blurred_self,'''
|
||||
new = '''
|
||||
"blurred_self": it.blurred,'''
|
||||
|
||||
|
||||
@@ -0,0 +1,227 @@
|
||||
"""Per-item blur storage — `.blurred` round-trips any rel, whoever writes it.
|
||||
|
||||
`.blurred` was one stripped rel per line, so a rel with a leading space could
|
||||
not survive a write: blurring " a.png" stored "a.png", and toggled the
|
||||
neighbour instead (heid bug-hunt on r2b merge 1, reported to booth-dev). It is
|
||||
now a JSON array, the `.seen` shape, read without following a link or blocking
|
||||
on a FIFO. The old line format is still READ, so nothing live changes until the
|
||||
next write upgrades it.
|
||||
|
||||
Two writers share the file: the service (the operator's per-item control) and
|
||||
`scripts/booth blur` (a session at post time). Both go through `booth.blur`,
|
||||
which is stdlib-only so the CLI can import it under the system python3.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import pathlib
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
sys.path.insert(0, str(pathlib.Path(__file__).parent.parent))
|
||||
|
||||
from booth.app import BLUR_FILE, create_app, read_blurred, set_blurred # noqa: E402
|
||||
from booth.items import booth_items # noqa: E402
|
||||
|
||||
PNG = b"\x89PNG\r\n\x1a\n"
|
||||
SCRIPT = pathlib.Path(__file__).parent.parent / "scripts" / "booth"
|
||||
|
||||
|
||||
def _booth(root: pathlib.Path, name: str, files: dict[str, bytes]) -> pathlib.Path:
|
||||
b = root / name
|
||||
b.mkdir()
|
||||
for rel, data in files.items():
|
||||
(b / rel).write_bytes(data)
|
||||
return b
|
||||
|
||||
|
||||
def _within(seconds: float, fn):
|
||||
"""Run fn in a thread and fail, rather than hang the suite, if it blocks."""
|
||||
out: dict = {}
|
||||
t = threading.Thread(target=lambda: out.setdefault("v", fn()), daemon=True)
|
||||
t.start()
|
||||
t.join(seconds)
|
||||
assert not t.is_alive(), f"{fn} blocked for over {seconds}s"
|
||||
return out["v"]
|
||||
|
||||
|
||||
# ---- the round-trip: the defect ---------------------------------------------
|
||||
|
||||
|
||||
def test_a_leading_space_rel_round_trips(tmp_path):
|
||||
"""Defeating change: storing rels line-stripped (the old format)."""
|
||||
set_blurred(tmp_path, " a.png", True)
|
||||
assert read_blurred(tmp_path) == {" a.png"}
|
||||
|
||||
|
||||
def test_unblurring_a_leading_space_rel_leaves_its_neighbour_blurred(tmp_path):
|
||||
"""The reported wrong-item write: " a.png" and "a.png" are two items, and
|
||||
toggling one must never move the other. (Unblurring the SPACED one would
|
||||
pass under the old format too — it was a no-op there — so this unblurs the
|
||||
plain one and asks whether the spaced one survived.)"""
|
||||
set_blurred(tmp_path, "a.png", True)
|
||||
set_blurred(tmp_path, " a.png", True)
|
||||
set_blurred(tmp_path, "a.png", False)
|
||||
assert read_blurred(tmp_path) == {" a.png"}
|
||||
|
||||
|
||||
def test_a_newline_in_a_rel_round_trips(tmp_path):
|
||||
"""A line format cannot hold one at all."""
|
||||
set_blurred(tmp_path, "two\nlines.png", True)
|
||||
assert read_blurred(tmp_path) == {"two\nlines.png"}
|
||||
|
||||
|
||||
def test_the_file_is_a_json_array_in_sorted_order(tmp_path):
|
||||
"""The `.seen` shape, and a stated order (invariant 6) so two writes of the
|
||||
same set are byte-identical."""
|
||||
set_blurred(tmp_path, "b.png", True)
|
||||
set_blurred(tmp_path, "a.png", True)
|
||||
assert json.loads((tmp_path / BLUR_FILE).read_text("utf-8")) == ["a.png", "b.png"]
|
||||
|
||||
|
||||
def test_emptying_the_set_removes_the_file(tmp_path):
|
||||
"""Unchanged: an empty marker is a lie by omission."""
|
||||
set_blurred(tmp_path, "a.png", True)
|
||||
set_blurred(tmp_path, "a.png", False)
|
||||
assert not (tmp_path / BLUR_FILE).exists()
|
||||
|
||||
|
||||
# ---- the legacy format: nothing live changes until it is written ------------
|
||||
|
||||
|
||||
def test_the_legacy_line_format_still_reads(tmp_path):
|
||||
"""Six live booths hold line-format files. Defeating change: a JSON-only
|
||||
reader, which would un-blur every one of them on deploy."""
|
||||
(tmp_path / BLUR_FILE).write_text("a.png\nsub/b.png\n\n")
|
||||
assert read_blurred(tmp_path) == {"a.png", "sub/b.png"}
|
||||
|
||||
|
||||
def test_a_legacy_rel_that_starts_with_a_bracket_still_reads(tmp_path):
|
||||
"""A line-format file whose first rel happens to begin with "[" is not
|
||||
JSON, and must fall back to lines rather than read as nothing."""
|
||||
(tmp_path / BLUR_FILE).write_text("[draft] a.png\nb.png\n")
|
||||
assert read_blurred(tmp_path) == {"[draft] a.png", "b.png"}
|
||||
|
||||
|
||||
def test_a_write_upgrades_a_legacy_file_and_keeps_its_rels(tmp_path):
|
||||
(tmp_path / BLUR_FILE).write_text("a.png\n")
|
||||
set_blurred(tmp_path, "b.png", True)
|
||||
assert json.loads((tmp_path / BLUR_FILE).read_text("utf-8")) == ["a.png", "b.png"]
|
||||
|
||||
|
||||
# ---- a planted file: never blocks, never follows ----------------------------
|
||||
|
||||
|
||||
def test_a_fifo_blur_file_does_not_block_the_read(tmp_path):
|
||||
"""read_blurred runs for every booth the Desk renders; a FIFO with no writer
|
||||
used to hang it — the outage class `.seen` was built against."""
|
||||
os.mkfifo(tmp_path / BLUR_FILE)
|
||||
assert _within(5, lambda: read_blurred(tmp_path)) == set()
|
||||
|
||||
|
||||
def test_a_symlinked_blur_file_is_not_followed_on_read(tmp_path):
|
||||
outside = tmp_path / "outside.json"
|
||||
outside.write_text('["a.png"]')
|
||||
b = tmp_path / "b"
|
||||
b.mkdir()
|
||||
(b / BLUR_FILE).symlink_to(outside)
|
||||
assert read_blurred(b) == set()
|
||||
|
||||
|
||||
def test_a_write_replaces_a_planted_symlink_rather_than_writing_through_it(tmp_path):
|
||||
outside = tmp_path / "outside.txt"
|
||||
outside.write_text("untouched")
|
||||
b = tmp_path / "b"
|
||||
b.mkdir()
|
||||
(b / BLUR_FILE).symlink_to(outside)
|
||||
set_blurred(b, "a.png", True)
|
||||
assert outside.read_text() == "untouched"
|
||||
assert not (b / BLUR_FILE).is_symlink()
|
||||
assert read_blurred(b) == {"a.png"}
|
||||
|
||||
|
||||
def test_malformed_json_array_contents_are_skipped_not_fatal(tmp_path):
|
||||
(tmp_path / BLUR_FILE).write_text('["a.png", 3, null, ["x"]]')
|
||||
assert read_blurred(tmp_path) == {"a.png"}
|
||||
|
||||
|
||||
# ---- the route: the operator's per-item control -----------------------------
|
||||
|
||||
|
||||
def test_the_blur_route_blurs_exactly_the_item_it_names(tmp_path):
|
||||
"""The route stripped `f` before writing, so the form for " a.png" blurred
|
||||
"a.png". Defeating change: `f.strip()` back in the route."""
|
||||
b = _booth(tmp_path, "g", {" a.png": PNG, "a.png": PNG})
|
||||
c = TestClient(create_app(tmp_path, ttl_hours=24, start_sweeper=False))
|
||||
r = c.post("/b/g/blur", data={"f": " a.png", "on": "1"}, follow_redirects=False)
|
||||
assert r.status_code == 303
|
||||
blurred = {it.rel: it.blurred for it in booth_items(b)}
|
||||
assert blurred == {" a.png": True, "a.png": False}
|
||||
|
||||
|
||||
# ---- the CLI: the other writer ----------------------------------------------
|
||||
|
||||
|
||||
def _cli(data: pathlib.Path, *args: str) -> subprocess.CompletedProcess:
|
||||
env = {**os.environ, "BOOTH_DATA_DIR": str(data), "BOOTH_URL": "http://booth.invalid"}
|
||||
return subprocess.run([str(SCRIPT), *args], capture_output=True, text=True, env=env, timeout=30)
|
||||
|
||||
|
||||
def test_the_cli_writes_the_format_the_service_reads(tmp_path):
|
||||
"""Both writers, one format. Defeating change: the CLI keeping its own
|
||||
grep/printf line writer, which appends a line to a JSON array."""
|
||||
b = _booth(tmp_path, "g", {" a.png": PNG, "a.png": PNG})
|
||||
set_blurred(b, "a.png", True) # the service wrote first
|
||||
r = _cli(tmp_path, "blur", "g", " a.png")
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert read_blurred(b) == {"a.png", " a.png"}
|
||||
r = _cli(tmp_path, "unblur", "g", " a.png")
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert read_blurred(b) == {"a.png"}
|
||||
|
||||
|
||||
def test_the_cli_unblurring_the_last_item_removes_the_file(tmp_path):
|
||||
b = _booth(tmp_path, "g", {"a.png": PNG})
|
||||
assert _cli(tmp_path, "blur", "g", "a.png").returncode == 0
|
||||
assert _cli(tmp_path, "unblur", "g", "a.png").returncode == 0
|
||||
assert not (b / BLUR_FILE).exists()
|
||||
|
||||
|
||||
def test_the_cli_still_refuses_a_dotdot_path(tmp_path):
|
||||
_booth(tmp_path, "g", {"a.png": PNG})
|
||||
r = _cli(tmp_path, "blur", "g", "../escape.png")
|
||||
assert r.returncode == 2
|
||||
assert not (tmp_path / "g" / BLUR_FILE).exists()
|
||||
|
||||
|
||||
# ---- one read of blur state per render (invariant 3) ------------------------
|
||||
|
||||
|
||||
def test_the_item_record_carries_its_own_blur_apart_from_the_booths(tmp_path):
|
||||
"""r2b's per-item control needs the item's OWN blur as well as the composed
|
||||
one. It came from a second `read_blurred` in build_gallery — a second reader
|
||||
of one file, which a write between the two could split. It is now resolved
|
||||
in `booth_items`, from the one read the composed fact already uses."""
|
||||
b = _booth(tmp_path, "g", {"a.png": PNG, "b.png": PNG})
|
||||
set_blurred(b, "a.png", True)
|
||||
(b / ".blurbooth").write_bytes(b"")
|
||||
got = {it.rel: (it.blurred, it.blurred_self) for it in booth_items(b)}
|
||||
assert got == {"a.png": (True, True), "b.png": (True, False)}
|
||||
|
||||
|
||||
def test_app_py_never_reads_the_blur_file_itself():
|
||||
"""Invariant 3, extended from route bodies to the whole module: blur state
|
||||
is read in `booth_items` and nowhere in app.py. Defeating change: the
|
||||
second `read_blurred` in build_gallery."""
|
||||
import ast
|
||||
src = pathlib.Path(__file__).parent.parent / "booth" / "app.py"
|
||||
calls = [
|
||||
n for n in ast.walk(ast.parse(src.read_text()))
|
||||
if isinstance(n, ast.Call) and getattr(n.func, "id", getattr(n.func, "attr", None)) == "read_blurred"
|
||||
]
|
||||
assert calls == []
|
||||
+7
-1
@@ -695,4 +695,10 @@ def test_unblurring_the_booth_keeps_per_item_choices(booth):
|
||||
run(data, "unblur", "b")
|
||||
|
||||
assert not (b / ".blurbooth").exists()
|
||||
assert (b / ".blurred").read_text().strip() == "a.png"
|
||||
# Read through the reader, not the bytes: `.blurred` became a JSON array
|
||||
# (the round-trip fix, operator-ruled 2026-09-23), and this test is about
|
||||
# the per-item choice surviving, not about the file's format.
|
||||
import sys
|
||||
sys.path.insert(0, str(pathlib.Path(__file__).parent.parent))
|
||||
from booth.blur import read_blurred
|
||||
assert read_blurred(b) == {"a.png"}
|
||||
|
||||
+1
-1
@@ -278,7 +278,7 @@ def test_as_dict_round_trips_through_json(tmp_path):
|
||||
# ---- the stdlib-only invariant (INV-5) --------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize("module", ["marks", "asks", "links", "manifest", "benches", "__init__"])
|
||||
@pytest.mark.parametrize("module", ["marks", "asks", "links", "manifest", "benches", "blur", "__init__"])
|
||||
def test_stdlib_only(module):
|
||||
"""INV-5. scripts/booth imports these under the system python3 with NO venv,
|
||||
through a `python3 -c` heredoc that no AST extractor can see — so nothing
|
||||
|
||||
Reference in New Issue
Block a user