From 018f1af5ff76a2893ee9da7457a8d5cf8cc2236f Mon Sep 17 00:00:00 2001 From: wtclaude Date: Thu, 10 Sep 2026 02:59:56 -0500 Subject: [PATCH] docs(link): phase 0 ran, and the fault it found is not a crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the Asset Bridge phase 0 spike (§16) against v8.md, and closes the last open decision. §4's choice to call ServUO's vendored `Ultima` 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 from inside the shard. But the spike was looking for the wrong kind of failure. `LoadStatic` and `LoadLand` decode out of a buffer that is reused, only ever grown, and filled by a `Read` whose return value is discarded — so a short, absent or out-of-bounds record does not throw, it renders the PREVIOUS asset. On the stock, unmodified client on this machine that is 22,102 ids whose index entry reads `lookup 0, length 0`, all of which the library returns a picture for. §1.1's "32,766 decoded" was counting these. New §4.5 states the rule that answers it — validate before calling — with the six checks phase 0 implemented, the eight deliberate defects they caught (seven of which the library rendered silently, including a verdata lookup past verdata.mul's own end, which `Verdata.Seek` bounds-checks nowhere), and the number that makes the boundary defensible: zero false refusals across 49,151 statics and 16,384 land tiles on a clean client. New §4.6: `FileIndex`'s UOP constructor ends `MulPath = uopPath`, so `artLegacyMUL.uop` wins outright and `art.mul` is never opened on a current client. Bounding an offset against the wrong file is not approximate, it is meaningless — the spike's first run refused 34,299 good statics that way, and every refusal read like a real finding. New §4.7: `Ultima.Gumps.GetGump(2)`, called once from inside a running shard, made the ServUO process disappear — no catch reached, no console line, the probe's checkpoint file the only record. §4.1's rule is earned. §17 now has nothing open: * item 4 — the default audience — SETTLED: an asset inherits the audience of the page that uses it. * item 5 is new: validate-before-calling, chosen ahead of the spike over a child-process extractor and over reversing §4, and confirmed by it. The dangerous failure turns out to be a wrong picture, which no containment strategy would have caught. §16 marks phase 0 done and adds the half it deliberately left unbuilt to phase 1: the animation path has no validator, and the patched client's wolf decoded something else in silence to prove it. Full measurements and the rig recipe live in servuo-plugins `tools/scaffolding/README.md`; the code is RunicGateway/servuo-plugins#27. - [x] AI-assisted — Claude Code (Opus 5) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4 --- link/v8.md | 134 +++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 121 insertions(+), 13 deletions(-) diff --git a/link/v8.md b/link/v8.md index 2964ad7..1ae3910 100644 --- a/link/v8.md +++ b/link/v8.md @@ -4,10 +4,12 @@ branches (`feat/asset-bridge-p`), 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, -built-in cliloc decoder, atlas cleanup); refined 2026-09-10 (one direction not five, ServUO's own -decoders, the UOP reader for player bodies, the libgdiplus requirement). §17 lists what is still -open. +**Status:** Design of record, **phase 0 complete**. Approved in principle 2026-09-09 +(architecture, asset scope, built-in cliloc decoder, atlas cleanup); refined 2026-09-10 (one +direction not five, ServUO's own decoders, the UOP reader for player bodies, the libgdiplus +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 <= .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 | -| 1 | The transport: `assets.sources`, flow control, the sidecar line cap, `EXTRACTOR_VERSION`, protocol bump | servuo-plugins, link | +| 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. **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. -**Item 4 is the only thing still open, and it does not block starting.** +Every item here is settled. Each is recorded because it changes numbers or obligations elsewhere +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 - every count in §11. -4. **§13: the default audience** for asset serving — inheriting the using page's audience is - proposed; the operator sets the policy either way. + `Race.AllRaces` rather than hardcoded. Recorded here because it changes every count in §11. + Phase 0 reproduced the twelve stock ids and the six that do not decode, exactly. +4. **§13: the default audience — settled 2026-09-10. An asset inherits the audience of the page + 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. -- 2.49.1