diff --git a/booth/app.py b/booth/app.py index 7555bcc..4184932 100644 --- a/booth/app.py +++ b/booth/app.py @@ -466,8 +466,12 @@ def record_view(booth: Path) -> None: # the utime is not decoration: the marker must read as NOW or the whole # mechanism is a file nobody's clock looks at. try: + # O_NONBLOCK: a FIFO planted at the marker would otherwise block this + # open forever with no reader — it cannot raise, so the swallow below + # never sees it, and the look costs the page after all (r3 heid bug + # hunt, hulda). Non-blocking, a reader-less FIFO fails with ENXIO. fd = os.open(booth / VIEW_MARKER, - os.O_WRONLY | os.O_CREAT | os.O_NOFOLLOW, 0o644) + os.O_WRONLY | os.O_CREAT | os.O_NOFOLLOW | os.O_NONBLOCK, 0o644) try: os.utime(fd) finally: @@ -1124,7 +1128,9 @@ def create_app( candidate = data_dir / name try: resolved = candidate.resolve() - except OSError: + except (OSError, ValueError): + # ValueError: an embedded NUL (`/b/g%00/…`) is not an OSError, and + # hostile input is a 404, never a 500 (r3 heid bug hunt, hulda) raise HTTPException(status_code=404, detail="no such booth") # resolved.parent must be the data dir itself — blocks symlink escape + nesting. if resolved.parent != data_dir or not resolved.is_dir(): @@ -1547,7 +1553,8 @@ def create_app( a, b = form.get("a"), form.get("b") if isinstance(a, str) and a and isinstance(b, str) and b: try: - ring = review_chain(booth_items(resolve_booth(name))) + booth = resolve_booth(name) + ring = _compare_ring(booth, booth_items(booth)) except HTTPException: ring = [] if a in ring and b in ring: @@ -1964,6 +1971,7 @@ def create_app( members = [r for r in ring if by_rel[r].group == item.group] group = {"key": item.group, "k": members.index(f) + 1, "n": len(members)} open_now = open_marks(marks) + cring = _compare_ring(booth, items) return templates.TemplateResponse( request, "view.html", { **common, @@ -1985,10 +1993,12 @@ def create_app( "is_last": pos == len(ring) - 1, "tray": [x for x in film if x["flagged"]], "back_url": f"/b/{quote(name, safe='')}/#item-{item.url}", - # R3 C2: this item against the NEXT in the ring (itself in a - # ring of one), keyed by rel like every compare URL + # R3 C2: this item against the NEXT in the compare ring + # (itself in a ring of one), keyed by rel like every compare + # URL. This item passed the containment check above, so it + # is in the compare ring. "compare_url": (f"/b/{quote(name, safe='')}/compare?a={quote(f, safe='/')}" - f"&b={quote(ring[(pos + 1) % len(ring)], safe='/')}"), + f"&b={quote(cring[(cring.index(f) + 1) % len(cring)], safe='/')}"), }) # .md renders, .txt/.log show as text — viewable in-booth, no download @@ -2007,6 +2017,28 @@ def create_app( url=f"/b/{quote(name, safe='')}/{quote(f, safe='/')}", status_code=307 ) + def _in_booth(booth: Path, rel) -> bool: + """The view route's rule for a path: it resolves, stays inside the + booth, and is a file. NEVER RAISES — it is asked once per ring item.""" + if not isinstance(rel, str) or not rel: + return False + try: + target = (booth / rel).resolve() + # the separator matters: a sibling booth `g-extra` shares `g`'s prefix + return str(target).startswith(str(booth) + os.sep) and target.is_file() + except (OSError, ValueError): + # ValueError: an embedded NUL — hostile input, never a 500 + return False + + def _compare_ring(booth: Path, items) -> list[str]: + """THE COMPARE RING (R3 C1): the review ring — item order, media only — + filtered to what a compare can open. `booth_items` follows symlinks, so + a link pointing outside the booth is in the review ring and compare + 404s it; every compare link, the review's Compare control and the flag + landing are built from THIS list, so none offers a pair that 404s (heid + bug hunt, 3 of 4).""" + return [r for r in review_chain(items) if _in_booth(booth, r)] + def _compare_side(booth: Path, ring: list[str], rel) -> str: """One side of a compare, or a 404 (R3 C1). A CONJUNCTION: the view route's resolve / containment / is_file check, AND membership of the @@ -2014,14 +2046,7 @@ def create_app( symlinks, so a link pointing outside the booth is IN the ring and only containment refuses it. The view's check alone is not enough — it renders a doc, and compare takes media only.""" - if not isinstance(rel, str) or not rel: - raise HTTPException(status_code=404, detail="no such item") - try: - target = (booth / rel).resolve() - except (OSError, ValueError): - # ValueError: an embedded NUL — hostile input, a 404, never a 500 - raise HTTPException(status_code=404, detail="no such item") - if not str(target).startswith(str(booth) + os.sep) or not target.is_file(): + if not _in_booth(booth, rel): raise HTTPException(status_code=404, detail="no such item") if rel not in ring: raise HTTPException(status_code=404, detail="no such item") @@ -2043,14 +2068,14 @@ def create_app( load, keeps them. They are `str`, not an int or a Literal, so an unknown value reads as the default and never as a 422. - The filmstrip is the review ring in RING ORDER (the item order filtered - to media — the review's `film`). + The filmstrip is the compare ring in RING ORDER (the review's `film`: + the item order filtered to media, less what compare cannot open). """ booth = resolve_booth(name) items = booth_items(booth) - ring = review_chain(items) - a = _compare_side(booth, ring, a) - b = _compare_side(booth, ring, b) + a = _compare_side(booth, review_chain(items), a) + b = _compare_side(booth, review_chain(items), b) + ring = _compare_ring(booth, items) # A look records both — below the 404s, so only a real pair counts. record_view(booth) record_seen(booth, a, items) diff --git a/booth/templates/base.html b/booth/templates/base.html index 986768a..1a34f85 100644 --- a/booth/templates/base.html +++ b/booth/templates/base.html @@ -859,7 +859,9 @@ .viewer.review.compare{grid-template-rows:auto minmax(0,1fr) auto auto} /* The sides share ONE set of rows (subgrid): a caption under A takes its height from both stages alike, never from A's alone — two stages of - different sizes would draw the same picture at two scales in Fit. */ + different sizes would draw the same picture at two scales in Fit. The + engine floor is subgrid (Chromium 117, Firefox 71, Safari 16); an older + engine drops the rows and the sides size on their own. */ /* The separator is a 1px column GAP showing the body's background, never a border on B: a border comes out of one side's width alone, and the two stages must be the same size to the pixel (heid code-review, hulda). */ diff --git a/booth/templates/compare.html b/booth/templates/compare.html index 4c574ed..c18ffbf 100644 --- a/booth/templates/compare.html +++ b/booth/templates/compare.html @@ -110,7 +110,10 @@ var i = href.indexOf('?'); if (i < 0) return href; var parts = href.slice(i + 1).split('#')[0].split('&').filter(function (p) { - return p && !/^(side|link)(=|$)/.test(p); + /* by the DECODED name: `%73ide=a` is `side=a` to the server */ + var n = p.split('=')[0]; + try { n = decodeURIComponent(n.replace(/\+/g, ' ')); } catch (e) {} + return p && n !== 'side' && n !== 'link'; }); if (active === 'a') parts.push('side=a'); if (!linked) parts.push('link=0'); diff --git a/docs/contracts/r3_compare.contract.md b/docs/contracts/r3_compare.contract.md index 63ea0bf..6d3f60f 100644 --- a/docs/contracts/r3_compare.contract.md +++ b/docs/contracts/r3_compare.contract.md @@ -57,9 +57,14 @@ assumptions: - **A look records both.** `record_view(booth)` once, and `record_seen` for `a` and then for `b`, below the 404s and gated on the records, as the view route gates it. Both calls never raise. +- **The compare ring** is the review ring filtered by that same conjunction: + item order, media only, less anything compare would 404 (an outside symlink + stays in the review ring). EVERY compare link is built from it: the strip, + the steps, the review's Compare control and the `back=compare` landing. So + no navigation offers a pair that 404s, and a step walks over such an item. - The response carries, per side: the rel, its quoted url, ordinal, kind, - caption, blurred, flagged and thumb. It also carries the ring as a filmstrip - in RING ORDER (the view route's `film`, one line in the route's docstring), + caption, blurred, flagged and thumb. It also carries the compare ring as a + filmstrip in RING ORDER (the view route's `film`, one line in the route's docstring), the linked and per-side step targets (C3), the back link (the review of `a`, which is also where `Esc` goes), and `ord_width`. @@ -256,9 +261,10 @@ assumptions: - **INV-1 — rel identity.** The pair is two rels, in the URL, always. Nothing about the pair is stored, and no ordinal ever addresses an item. -- **INV-2 — ring only.** Both sides are media in the review ring. Every - server-computed link (the steps, the filmstrip, the flag landing) stays inside - it. +- **INV-2 — ring only.** Both sides are media in the review ring that pass the + view route's containment. Every server-computed link (the steps, the + filmstrip, the review's Compare control, the flag landing) stays inside the + compare ring (C1). - **INV-3 — no new storage and no new mark.** The judgment is the existing flag, through the existing route and the existing in-place path. - **INV-4 — JS-off parity.** Without JS (and so without the head script that diff --git a/tests/mutations/r3.toml b/tests/mutations/r3.toml index 3dd35b1..b30c0c2 100644 --- a/tests/mutations/r3.toml +++ b/tests/mutations/r3.toml @@ -10,11 +10,9 @@ label = "C1 the conjunction loses containment (an outside symlink in the ring op file = "booth/app.py" test = "tests/test_compare.py::test_an_outside_symlink_in_the_ring_is_404" old = ''' - if not str(target).startswith(str(booth) + os.sep) or not target.is_file(): - raise HTTPException(status_code=404, detail="no such item") - if rel not in ring:''' + return str(target).startswith(str(booth) + os.sep) and target.is_file()''' new = ''' - if rel not in ring:''' + return target.is_file()''' [[mutation]] label = "C1 the conjunction loses the ring (a doc or a sidecar opens as a side)" @@ -50,14 +48,12 @@ test = "tests/test_compare.py::test_a_look_records_both_seen" old = ''' booth = resolve_booth(name) items = booth_items(booth) - ring = review_chain(items) - a = _compare_side(booth, ring, a)''' + a = _compare_side(booth, review_chain(items), a)''' new = ''' booth = resolve_booth(name) record_view(booth) items = booth_items(booth) - ring = review_chain(items) - a = _compare_side(booth, ring, a)''' + a = _compare_side(booth, review_chain(items), a)''' [[mutation]] label = "C6 compare does not carry data-booth (Reveal all and its restore bail)" @@ -118,8 +114,8 @@ new = ''' side_a = side not in ("", "b")''' label = "C2 the review's Compare does not wrap (the last item compares with itself)" file = "booth/app.py" test = "tests/test_compare.py::test_the_review_offers_compare_with_the_next_item" -old = ''' f"&b={quote(ring[(pos + 1) % len(ring)], safe='/')}"),''' -new = ''' f"&b={quote(ring[min(pos + 1, len(ring) - 1)], safe='/')}"),''' +old = ''' f"&b={quote(cring[(cring.index(f) + 1) % len(cring)], safe='/')}"),''' +new = ''' f"&b={quote(cring[min(cring.index(f) + 1, len(cring) - 1)], safe='/')}"),''' # ---- C5: the regions and the JS-off flag landing @@ -480,11 +476,9 @@ label = "C1 containment is a bare prefix (a sibling booth sharing the name opens file = "booth/app.py" test = "tests/test_compare.py::test_an_outside_symlink_in_the_ring_is_404" old = ''' - if not str(target).startswith(str(booth) + os.sep) or not target.is_file(): - raise HTTPException(status_code=404, detail="no such item")''' + return str(target).startswith(str(booth) + os.sep) and target.is_file()''' new = ''' - if not str(target).startswith(str(booth)) or not target.is_file(): - raise HTTPException(status_code=404, detail="no such item")''' + return str(target).startswith(str(booth)) and target.is_file()''' [[mutation]] label = "C1 the strip is not in ring order" @@ -511,6 +505,51 @@ test = "tests/test_compare_browser.py::test_a_flags_A_in_place_and_the_stages_su old = '''