diff --git a/crates/benilla-ui/src/script/stdlib.rs b/crates/benilla-ui/src/script/stdlib.rs index 116bfaa2..6c018fac 100644 --- a/crates/benilla-ui/src/script/stdlib.rs +++ b/crates/benilla-ui/src/script/stdlib.rs @@ -235,6 +235,42 @@ do function geterrorhandler() return handler end end +-- ── the taint-system globals, **beyond 1.12** ──────────────────────────────────────── +-- 1.12 has no taint, so there is nothing here to make secure; what these are for is that modern +-- library code **upvalues them at load time** and then calls them on every dispatch. The case +-- that reached a player: Ace3's own `CallbackHandler-1.0` MINOR 8 takes +-- `local securecallfunction = securecallfunction` on its line 10 and calls it for every handler +-- of every event (`Dispatch`, line 19). With the global absent that upvalue is nil, so the first +-- callback any addon shipping that copy fires raises `attempt to call upvalue +-- 'securecallfunction'` — and the copy is vendored by OpenMapClassic, Ace3 addons generally, and +-- anything else carrying a post-2022 Ace. +-- +-- On a client with no taint, `securecallfunction(f, ...)` is `f(...)` made **protected**: that is +-- the half of its contract that is not about taint, and the half library code depends on — one +-- handler raising must not stop the registry walking the rest. Errors go to `geterrorhandler()`, +-- as the reference's own secure dispatch does, so an addon bug still reaches the log instead of +-- being swallowed. `issecure()` answers true because nothing here can be tainted, and +-- `forceinsecure()` has nothing to do. +do + -- the varargs-preserving `pcall` idiom: Lua 5.1 cannot count return values any other way + local function pass(ok, ...) + if not ok then geterrorhandler()((...)) return end + return ... + end + function securecallfunction(func, ...) + if type(func) ~= "function" then return end + return pass(pcall(func, ...)) + end + -- the older spelling, which also takes a global's NAME in place of the function + function securecall(func, ...) + if type(func) == "string" then func = _G[func] end + if type(func) ~= "function" then return end + return pass(pcall(func, ...)) + end + function issecure() return true end + function forceinsecure() end +end + -- ── _G accessors (getglobal/setglobal: _G[name] get/set) ─────────────────────────────── function getglobal(name) return _G[name] end function setglobal(name, value) _G[name] = value end diff --git a/crates/benilla-ui/src/script/tests/reference_surface.rs b/crates/benilla-ui/src/script/tests/reference_surface.rs index 2b713ada..ae036fb8 100644 --- a/crates/benilla-ui/src/script/tests/reference_surface.rs +++ b/crates/benilla-ui/src/script/tests/reference_surface.rs @@ -57,6 +57,18 @@ fn allowed_beyond_1_12() -> HashSet<&'static str> { // TurtleWoW's native arena-score request: a no-op here (its reply is parsed by TurtleWoW's // client, and benilla has no arena scoreboard), so its WorldStateFrame.lua:217 cannot raise. "RequestArenaScoreInfo", + // ── the taint system, which 1.12 does not have at all ── + // Not for taint — there is none here — but because modern library code **upvalues these + // at load time** and then calls them on every dispatch. Ace3's `CallbackHandler-1.0` + // MINOR 8 takes `local securecallfunction = securecallfunction` on its line 10 and calls + // it for every handler of every event, so without the global the first callback any addon + // shipping that copy fires raises `attempt to call upvalue 'securecallfunction'` — it took + // the addon corpus's UI-probe column from 86/87 to 85/87 the moment OpenMapClassic's own + // vendored copy fired one. See `stdlib.rs`. + "securecallfunction", + "securecall", + "issecure", + "forceinsecure", // ── WoW API past 1.12 ── // Only `SubmitChatInput` is called from our UI files (`ScriptLogFrame.xml`'s slash // commands); each of the rest awaits its 1.12 equivalent, removal or a reason. diff --git a/crates/benilla-ui/src/script/tests/stdlib.rs b/crates/benilla-ui/src/script/tests/stdlib.rs index d4d124fb..92956062 100644 --- a/crates/benilla-ui/src/script/tests/stdlib.rs +++ b/crates/benilla-ui/src/script/tests/stdlib.rs @@ -425,13 +425,20 @@ fn the_rest_of_the_bare_globals() { -- string.match/gmatch, and claiming otherwise misleads an addon that feature-detects. assert(strmatch == nil and gmatch == nil and strrev == nil) assert(strlenutf8 == nil and strcmputf8i == nil) - -- `securecall`/`issecure` stay absent: both are the modern restricted-execution system, - -- which has no 1.12 counterpart and nothing here implements. `hooksecurefunc` is NOT in - -- this list any more — it is a deliberate addon-compat addition (ClassicAPI backports it, - -- and a large share of real addons call it), listed with its reason in - -- `reference_surface.rs`'s `allowed_beyond_1_12` and built in - -- `script/compat/classic_api/hooks.rs`. See `docs/ADDON_COMPAT.md`. - assert(securecall == nil and issecure == nil) + -- `securecall`/`securecallfunction`/`issecure`/`forceinsecure` used to be asserted ABSENT + -- here, on the reasoning that the restricted-execution system has no 1.12 counterpart and + -- nothing here implements it. That reasoning was half right, and the half it missed cost + -- the addon corpus a column: modern library code **upvalues `securecallfunction` at load + -- time** and calls it on every dispatch — Ace3's `CallbackHandler-1.0` MINOR 8, line 10 + -- and line 19 — so the first callback any addon shipping that copy fires raises + -- `attempt to call upvalue 'securecallfunction'`. There is still no taint to make secure; + -- what is implemented is the other half of the contract, a protected call reporting + -- through `geterrorhandler()`, so one raising handler does not stop a registry walking the + -- rest. Same shape as `hooksecurefunc` below: a deliberate addon-compat addition, listed + -- with its reason in `reference_surface.rs`'s `allowed_beyond_1_12`, and pinned by + -- `the_taint_globals_are_protected_calls_that_keep_walking_after_one_raises`. + assert(type(securecallfunction) == "function" and type(securecall) == "function") + assert(issecure() == true and forceinsecure() == nil) assert(type(hooksecurefunc) == "function") -- math family @@ -611,3 +618,57 @@ fn a_runscript_error_reaches_the_handler_and_not_the_caller() { "a compile failure is reported under the same name, undecorated: {errs:?}" ); } + +/// `securecallfunction` is upvalued at load time by Ace3's `CallbackHandler-1.0` MINOR 8 (its +/// line 10) and called for every handler of every event (`Dispatch`, line 19). Absent, that +/// upvalue is nil and the first callback any addon shipping that copy fires raises — measured +/// against OpenMapClassic's own vendored copy, which took the corpus's "survived a UI probe" +/// column from 86/87 to 85/87 the moment anything fired through it. +/// +/// There is no taint here to make secure, so what is implemented is the other half of the +/// contract and the half library code leans on: a protected call whose failure reaches +/// `geterrorhandler()` and lets the registry walk the rest of its handlers. +#[test] +fn the_taint_globals_are_protected_calls_that_keep_walking_after_one_raises() { + let mut s = script(); + s.run( + "BenillaSeen = {} + local function ok(a, b) table.insert(BenillaSeen, a .. b) return a, b, nil, 4 end + local function bad() error('handler blew up') end + BenillaR = { securecallfunction(ok, 'x', 'y') } + securecallfunction(bad) + securecallfunction(ok, 'p', 'q') + -- the older spelling also takes a global's NAME + BenillaNamed = 0 + function BenillaByName(n) BenillaNamed = n end + securecall('BenillaByName', 7) + securecall(ok, 'm', 'n') + -- neither spelling may raise on a non-function + securecallfunction(nil) + securecall('BenillaNoSuchGlobal') + BenillaSecure = issecure() + forceinsecure()", + ) + .expect("a raising handler must not propagate to the caller"); + + assert_eq!( + s.eval::>("return BenillaSeen").unwrap(), + vec!["xy", "pq", "mn"], + "the call after the raising one still ran, which is the whole point" + ); + // Return values pass through, trailing nils and all — `CallbackHandler` discards them, but a + // caller that reads one must get its own answer back. + assert_eq!( + s.eval::<(String, String)>("return BenillaR[1], BenillaR[2]") + .unwrap(), + ("x".to_string(), "y".to_string()), + ); + assert_eq!(s.eval::("return BenillaNamed").unwrap(), 7); + assert!( + s.eval::("return BenillaSecure").unwrap(), + "nothing on a client with no taint system can be insecure" + ); + let errs = s.take_errors(); + assert_eq!(errs.len(), 1, "exactly the one raise, reported: {errs:?}"); + assert!(errs[0].contains("handler blew up"), "{errs:?}"); +} diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index a7455694..29d5d8e2 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -4091,3 +4091,68 @@ Quality was checked rather than assumed: four THICKOUTLINE rows at the ladder's (0.889× minify, 1.118× magnify), same scene, both binaries. A control band with no rig text is **pixel-identical**, the tight bounding boxes of the 72 px and 90 px rows are identical to the pixel, and the text bands differ by 4.6–6.0% RMSE — edge antialiasing, invisible at 1:1. + +## The stitched map never said which zone you were looking at + +Blizzard's map writes the zone's name in the window chrome, because the window only ever shows one +map. The stitched map is a continuous canvas with no "current map" to name, and measured over a +capture sweep at zoom 1, 2, 3, 4, 5, 6, 10, 20 and 40, the only lettering on screen is what the art +itself paints: the world sheet's `KALIMDOR` / `EASTERN KINGDOMS`, off the window's edge by zoom 3; +each continent sheet's painting of **its own** name (this client's continent art paints no zone +names at all — 1.12's `WorldMapContinent` sheets are coloured zone blobs and a title); and each +zone sheet's sub-area names, `MENETHIL HARBOR`, `DUN ALGAZ`, which name a place inside a zone and +never the zone. **From about zoom 3.5 to about zoom 10 there is no zone name anywhere on screen**, +and that is the whole band a player pans around in. + +Upstream fills this with `ns.MapLabels` and `Stitch.lua`'s `Letter()`; neither is usable here. +`Data/Labels.lua` is empty in this fork because the positions were hand-placed against hand-cleaned +art this client does not have, and `Letter()` builds one FontString per glyph plus an eight-tap +halo ring per glyph — its own comment admits every one is moved on each frame of a pan and redrawn +on each frame of a zoom, which is the glyph-sheet defect two sections up, by construction. + +`Map/ZoneNames.lua` is one plain outlined FontString per zone instead. Three things make it cheap: + +- **It hangs off the view, not the canvas.** The holder sits on `WorldMapFrame.ScrollContainer`, + whose effective scale measured **1.0000 at every zoom** while the canvas measured 3.5261 at zoom + 5. A font height in view units is therefore a fixed device ppem, and `(face, glyph, ppem, + outline)` keys one cell set for the whole session. Measured: **size 14 at every zoom in the + sweep**, and zero `glyph sheet full` lines. It is the same move `MapCanvas.lua` already makes + with `WorldMapFrameAreaFrame`. +- **Positions still ride the canvas**, anchored to `WorldMapButton` with the `button / holder` + scale factor `Map/StockArrow.lua` documents, so a pan is carried by the layout for free and only + a zoom needs the callback. +- **Two zoom bands, both read off the sweep.** In at `{ 3.4, 5.0 }`, a little ahead of + `MUTE_FADE = { 4, 7 }` so the name is already there when the art stops helping; out at + `{ 10, 16 }`, where the zone sheet's own lettering has taken over (`MENETHIL HARBOR` is fully + readable at zoom 10 and mush at 5). A per-zone band in the style of `CITY_FADE` was tried and + rejected: The Barrens fills the view at zoom 4.4, so its name would fade out before it finished + fading in, and Moonglade fills it at 19.3 and would hang on past zoom 40. + +The label is placed at the zone's rect centre **clamped into the part of that rect which is on +screen**, inset by half the measured string. Pinning to the raw centre has two failures a player +meets at ordinary zooms: a zone whose centre is off screen gets no name at all — between zoom 8 and +10 that can be the very zone being looked at — and a centre near the canvas edge gets the word +sliced, because the view clips its children. Measured at zoom 9 over Dun Morogh: **2 labels before +the clamp, 6 after**, none sliced. + +Cities are excluded (that is `Stitch.lua`'s own `S.stats.cities`), the whole thing is one +`zoneNames` toggle, and with it off the shot is byte-identical to one taken with the file absent. +Honest remainders: a name can overlap the sheet's own lettering (`DUN MOROGH` over `KHARANOS` at +zoom 13, at 50% alpha), and during an easing zoom a label lags radially by a frame or two because +the view callback is throttled to 0.15 s — pans are exact. + +### `securecallfunction`, and why an empty taint system still needs one + +Wiring that file up took the corpus's "survived a UI probe" column from 86/87 to **85/87**, and the +failure was not in it: `OpenMapClassic/Libs/CallbackHandler-1.0/CallbackHandler-1.0.lua:19`, +`attempt to call upvalue 'securecallfunction' (a nil value)`. Ace3's modern `CallbackHandler` +upvalues that global on its line 10 and calls it for **every handler of every event**, so the first +callback any addon shipping a post-2022 Ace fires raises. It had simply never fired before — the +callbacks were dead for the other reason, two sections up. + +1.12 has no taint, so there is nothing here to make secure. But the half of the contract that is +not about taint is the half library code leans on: it is a **protected** call, so one handler +raising does not stop the registry walking the rest. `securecallfunction`, `securecall` (which also +takes a global's name), `issecure` and `forceinsecure` now live beside `seterrorhandler` in +`stdlib.rs`, with failures going to `geterrorhandler()` exactly as the reference's own secure +dispatch does — so an addon bug still reaches the log rather than being swallowed. Back to 86/87.