Merge pull request 'docs(link): phase 0 ran, and the fault it found is not a crash' (#235) from docs/asset-bridge-p0 into main

Reviewed-on: #235

link/v8.md

@@ -4,10 +4,12 @@
 branches (`feat/asset-bridge-p<n>`), and conversation. The protocol number is **8** and this file is
 `docs/link/v8.md`.
 
-**Status:** Design of record. Approved in principle 2026-09-09 (architecture, asset scope,
+**Status:** Design of record, **phase 0 complete**. Approved in principle 2026-09-09
-built-in cliloc decoder, atlas cleanup); refined 2026-09-10 (one direction not five, ServUO's own
+(architecture, asset scope, built-in cliloc decoder, atlas cleanup); refined 2026-09-10 (one
-decoders, the UOP reader for player bodies, the libgdiplus requirement). §17 lists what is still
+direction not five, ServUO's own decoders, the UOP reader for player bodies, the libgdiplus
-open.
+requirement). The phase 0 spike ran on 2026-09-10 and its findings are §4.5-§4.7 -- §4's decision
+stands, and the response to a malformed record is now measured rather than proposed. **Nothing in
+§17 is open.**
 **Supersedes the manual half of:** [`../website/UOFIDDLER.md`](../website/UOFIDDLER.md),
 [`../website/CLILOCS.md`](../website/CLILOCS.md) §Converting,
 [`../website/SPAWN_ATLAS.md`](../website/SPAWN_ATLAS.md) §Artwork and §Configuring the tree.
@@ -266,6 +268,97 @@ later.
 
 ---
 
+### 4.5 What phase 0 measured, and the rule it produced
+
+Phase 0 ran §4's decoders from inside a live ServUO 57.4 against a client broken in 21 catalogued
+ways ([`servuo-plugins/tools/scaffolding/README.md`](https://gitea.whitlocktech.com/RunicGateway/servuo-plugins/src/branch/main/tools/scaffolding/README.md)
+carries the full results). §4's decision **stands** — nothing faulted on a path this protocol
+calls, and §9's cliloc reader reproduced UOFiddler's 123,490-entry table byte for byte in 218 ms.
+
+But the spike was looking for the wrong kind of failure, and found a worse one.
+
+**A malformed record does not usually throw. It renders the previous asset.** `LoadStatic` and
+`LoadLand` decode out of `m_StreamBuffer`, which is reused across calls, only ever grown, and
+filled by a `stream.Read` whose return value is discarded. So a record that is short, absent or
+out of bounds produces a real bitmap of whatever was decoded last — reported as success by every
+count in the library and undetectable by anything downstream.
+
+On the **stock, unmodified** client on this machine that is **22,102 ids**: 9,962 statics and
+12,140 land tiles whose index entry reads `lookup 0, length 0`. `FileIndex.Seek` rejects
+`lookup < 0` and `length < 0`, and zero is neither, so it treats an empty slot as a hit. A bulk
+import that trusted the library would have written 22,102 duplicate images into the site under
+ids that have no art. §1.1's "32,766 decoded" was counting these.
+
+**This is specific to the UOP path** (see §4.6), because `artidx.mul` stores `-1` for an absent
+record where an unmapped UOP slot is a zeroed struct.
+
+So the rule, and it is the deliverable phase 1 inherits:
+
+> **Validate before calling.** The extractor judges an index entry — and, for statics, the record
+> header and row table behind it — *before* handing the id to `Ultima`. A record that fails is
+> reported as absent, never decoded.
+
+The checks, all of which phase 0 implemented and measured as `BridgeAssetValidator`:
+
+| Check | The shape it stops |
+|---|---|
+| `lookup >= 0`, `length > 0` | the 22,102 empty slots above |
+| `lookup + length <= <data file>.Length` | **`Seek` checks that a record *starts* inside the file and never that it *ends* inside it**; a short read then decodes the previous asset |
+| the same bound against `verdata.mul` for a patched entry | `Verdata.Seek` is bounds-checked **nowhere at all** |
+| land records are at least 2,024 bytes | `LoadLand` reads exactly that many whatever the length says |
+| declared width and height within a ceiling | `LoadStatic` allocates `new Bitmap(width, height)` from two bytes in the file — phase 0 got a ~128 MB allocation out of an edit, and the same field can ask for 8 GB |
+| walk the row table and every run, bounded | `LoadStatic`'s two guards bound the **write** into the bitmap and nothing bounds the **read** out of the record |
+
+Measured against the patched client, this refused all eight record-level defects, seven of which
+the library rendered without raising anything. Measured against the stock client it refused
+**nothing** across 49,151 statics and 16,384 land tiles. That second number is the one that makes
+the boundary defensible: a checker that refuses real art would be worse than no checker.
+
+Two more ways an id with no art yields a picture, both of which the extractor must handle itself:
+`Art.GetStatic(id, false)` **throws** `IndexOutOfRangeException` above the index's own ceiling
+(16,385 ids in a full sweep), and `Art.GetStatic(id)` with the default `checkmaxid: true` is worse
+— `GetLegalItemID` maps an out-of-range id to **0** and returns **item 0's picture**. Take the
+ceiling from the index that was opened, and pass `checkmaxid: false` so an overrun is loud.
+
+**The animation path has none of this yet**, and phase 0 proved it needs it: the patched client's
+verdata entry for body 34 points past `verdata.mul`'s end and the wolf still "decoded", counted
+among the 1,144 successes while rendering something else. `GetAnimation` additionally allocates
+`new int[frameCount]` straight from a file-supplied int. Extending the validator to animations is
+phase 1 work, not a phase 9 tidy-up.
+
+### 4.6 The UOP wins outright, and `art.mul` is never opened
+
+`FileIndex`'s UOP constructor ends with a bare `MulPath = uopPath`. **When `artLegacyMUL.uop` is
+present it wins, and `art.mul` / `artidx.mul` are not opened at all.** Every current client ships
+the UOP, so this is the normal case and not an edge one.
+
+It matters twice.
+
+**For us:** an index entry's `lookup` is an offset into whichever file `FileIndex` resolved, so any
+bound taken against `art.mul` while the index holds UOP offsets is not approximate — it is
+meaningless. Phase 0's first run refused **34,299 perfectly good statics** for "declaring
+10533x2085" on exactly that mistake, and every one of those refusals read like a real finding. The
+extractor must resolve the data file with `FileIndex`'s own precedence, not by name.
+
+**For operators:** a shard that adds custom art to `art.mul` while the UOP is still in place gets
+nothing, silently. Worth a line in the admin surface's diagnostics (§14) rather than leaving an
+operator to discover it as "my art did not import".
+
+### 4.7 The gump crash, reproduced where it counts
+
+§4.1 argued from source that the access violation lives on the `hasExtra: true` branch only `Gumps`
+reaches. Phase 0 called `Ultima.Gumps.GetGump(2)` **once, from inside a running shard**: the ServUO
+process disappeared. No exception line, no `catch` reached, no shutdown, nothing in the console —
+the probe's checkpoint file, written before the call, was the entire record of what happened.
+
+`AccessViolationException` is a corrupted-state exception that .NET Framework 4.8 does not deliver
+to ordinary handlers, so on a live shard this is a crash with players on it and **there is no
+in-process defence**. §4.1's "nothing in this protocol calls `Ultima.Gumps`" is therefore an earned
+safety rule rather than a scoping preference, and adding gump art later means fixing that path
+first, deliberately.
+
+---
+
 ## 5. Addressing: one key for every asset
 
 Every asset the bridge can serve is named by a single string key, and the key is the cache key,
@@ -614,8 +707,8 @@ disagree, so a split bump means the next bundle silently fails to compose.
 
 | # | Scope | Repos |
 |---|---|---|
-| 0 | Spike: the vendored decoders driven **from inside a running ServUO**, over a deliberately patched client — statics, land, bodies, and the Mythic cliloc against UOFiddler's output. What it is looking for is a fault on a path we call (§4.2) | servuo-plugins |
+| 0 | **DONE 2026-09-10.** Spike: the vendored decoders driven from inside a running ServUO, over a client broken 21 ways. §4 stands; the finding was 22,102 wrong pictures on a *stock* client, and the validator that answers them (§4.5-§4.7) | servuo-plugins |
-| 1 | The transport: `assets.sources`, flow control, the sidecar line cap, `EXTRACTOR_VERSION`, protocol bump | servuo-plugins, link |
+| 1 | The transport: `assets.sources`, flow control, the sidecar line cap, `EXTRACTOR_VERSION`, protocol bump. **Plus §4.5's validator promoted into the overlay, extended to animations** — phase 0 left that half unbuilt and proved it is needed | servuo-plugins, link |
 | 2 | Clilocs end to end; retire the converter and `UOFIDDLER.md` §Part 1 | all |
 | 3 | Body resolution (§8) + the 1,144-body catalogue; `shard_spawn_creatures.art` filled | servuo-plugins, module-uo |
 | 4 | **The UOP animation decoder (§4.3)** — the six player bodies the legacy path cannot reach, and the never-sweep-file-types rule enforced in code | servuo-plugins |
@@ -641,20 +734,35 @@ because §4.3's whole point is that this failure mode produces confident, wrong
 
 ## 17. Decisions
 
-Items 1–3 are settled and recorded here because each one changes numbers elsewhere in the document.
+Every item here is settled. Each is recorded because it changes numbers or obligations elsewhere
-**Item 4 is the only thing still open, and it does not block starting.**
+in the document.
 
 1. **§4: settled 2026-09-10 — call ServUO's vendored `Ultima`,** with one exception added the same
    day: **§4.3's UOP animation decoder**, scoped to the player bodies the legacy path cannot reach
    and to nothing else (phase 4). The crash is confined to the `hasExtra: true` branch that only
-   `Gumps` reaches, and nothing here calls `Gumps`.
+   `Gumps` reaches, and nothing here calls `Gumps`. **Phase 0 confirmed both halves in the real
+   host process** — nothing faulted on a path we call, and one `GetGump(2)` killed the shard
+   outright (§4.7).
 2. **§4.4: settled 2026-09-10 — libgdiplus is a stated requirement** on Linux shard hosts, with
    all three answers taken rather than one: it goes in `SHARD_PREREQS.md`, the installer's `doctor`
    checks it, and its absence degrades to a named `NO_IMAGING` status instead of an error. Windows
    hosts are unaffected.
 3. **§5.1/§5.2: direction — settled 2026-09-10.** Player character bodies use index 0, everything
    else index 1, direction is not in the key, and the player-body set is enumerated from
-   `Race.AllRaces` rather than hardcoded. Nothing outstanding; recorded here because it changes
+   `Race.AllRaces` rather than hardcoded. Recorded here because it changes every count in §11.
-   every count in §11.
+   Phase 0 reproduced the twelve stock ids and the six that do not decode, exactly.
-4. **§13: the default audience** for asset serving — inheriting the using page's audience is
+4. **§13: the default audience — settled 2026-09-10. An asset inherits the audience of the page
-   proposed; the operator sets the policy either way.
+   that uses it.** A bestiary that is already anonymous keeps anonymous pictures; a staff-only
+   screen's art is staff-only. The operator can still set the policy explicitly, and the
+   `requireFeature` gate with its 404-not-403 behaviour is unchanged. The alternative — one flat
+   audience for all asset serving — was rejected because it necessarily disagrees with some page
+   that uses it, in one direction or the other, and the disagreement is silent either way.
+5. **§4.5: the response to a malformed record — settled 2026-09-10, and now measured. Validate
+   before calling.** The extractor judges an index entry, and for statics the record behind it,
+   before handing the id to `Ultima`; a record that fails is reported absent and never decoded.
+   Chosen ahead of phase 0 over two alternatives — extracting in a child process (much stronger
+   containment, a much larger change to §2 and phase 1) and reversing §4 to write our own decoders
+   (weeks, per §2.1). Phase 0 then found the shape that settles it: the dangerous failure is not a
+   crash a child process would contain, it is a **wrong picture** that no containment strategy
+   would have caught, on **22,102 ids of a stock client**. See §4.5 for the checks and the
+   false-refusal measurement that says the boundary is in the right place.