From f7fc0017a6a6f31a4ae725f9f391d8dc63e3d9cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 26 Aug 2026 00:01:33 +0000 Subject: [PATCH] fix(core): evict cached MediaElementSource on src mutation, soften enforcement-point claims --- packages/core/src/runtime/webAudioRoute.ts | 15 ++++++ .../src/runtime/webAudioTransport.test.ts | 52 +++++++++++++++++++ .../core/src/runtime/webAudioTransport.ts | 42 +++++++++++++-- 3 files changed, 105 insertions(+), 4 deletions(-) diff --git a/packages/core/src/runtime/webAudioRoute.ts b/packages/core/src/runtime/webAudioRoute.ts index f1eeb6e04..0866e8c60 100644 --- a/packages/core/src/runtime/webAudioRoute.ts +++ b/packages/core/src/runtime/webAudioRoute.ts @@ -34,6 +34,21 @@ import type { RuntimeJson } from "./types"; * `web-audio` here, correctly or not. Nothing in this codebase feeds * `createMediaElementSource` from a `srcObject` element today, so this is * recorded as a boundary rather than fixed. + * + * Second known gap, same shape: `isCorsSilenced` judges the RAW url string — + * the same-origin URL the author wrote, or whatever the browser resolved into + * `currentSrc` — not wherever a server-side redirect chain actually lands. + * A same-origin URL that 302s to a cross-origin CDN reads as `web-audio` here + * and gets a real `createMediaElementSource` node; whether that node is + * silent then depends on the redirect target's CORS headers, which this + * classifier never sees (following the chain to inspect the final response + * would turn a pure, synchronous verdict — needed on every schedule call — + * into an async fetch). A cross-origin URL that redirects back to same-origin + * has the opposite miss: classified `decode-only` and sent down the fetch + * fallback when Web Audio capture would have worked fine either way. Not + * fixed for the same reason as `srcObject` — no caller in this codebase + * routes media through a redirecting URL today — but worth knowing before + * trusting this classifier's verdict for one that does. */ export type WebAudioMediaRoute = /** Same-origin, CORS-opted-in, or a scheme the check doesn't apply to. */ diff --git a/packages/core/src/runtime/webAudioTransport.test.ts b/packages/core/src/runtime/webAudioTransport.test.ts index edc61639c..db32530e7 100644 --- a/packages/core/src/runtime/webAudioTransport.test.ts +++ b/packages/core/src/runtime/webAudioTransport.test.ts @@ -202,6 +202,58 @@ describe("WebAudioTransport", () => { expect(scheduled).not.toBeNull(); expect(mock.ctx.createMediaElementSource).toHaveBeenCalledWith(el); }); + + // R2 finding: `_mediaElementSources` is keyed by element identity, not by + // asset. A cache hit alone said nothing about the element's CURRENT + // resource, so a pooled element reused for a new clip kept handing back + // the OLD (same-origin) node — and its stale `web-audio` verdict — after + // `src` moved to a cross-origin asset with no `crossorigin` opt-in. Only + // `destroy()` ever cleared the cache, so this silenced the element for the + // rest of the session. + it("stops returning the cached node once the same element's src moves cross-origin", async () => { + const { transport, mock, gen: gen1 } = setupTransport(100); + vi.spyOn(console, "info").mockImplementation(() => {}); + const el = document.createElement("audio"); + el.setAttribute("src", "/assets/vo.mp3"); + + const first = await transport.scheduleMediaElementPlayback(el, 0, 0, 0, 1, gen1, 1); + expect(first).not.toBeNull(); + expect(mock.ctx.createMediaElementSource).toHaveBeenCalledTimes(1); + + transport.stopAll(); + // `stopAll()` itself disconnects the transient graph (see "disconnects + // the transient graph on stop but keeps the cached native source + // reusable" above) without evicting the cache — clear the spy so the + // assertion below is about the FIX's own eviction, not that call. + mock.mediaElementSourceNode.disconnect.mockClear(); + el.setAttribute("src", "https://cdn.example.com/reused-clip.mp3"); + const gen2 = transport.startGeneration(); + const second = await transport.scheduleMediaElementPlayback(el, 0, 0, 0, 1, gen2, 1); + + expect(second).toBeNull(); + // The one-way door means a fresh node can't be built either — the + // fix's job is to stop HANDING BACK the stale one, not to conjure a + // new node over a src that was never eligible. + expect(mock.ctx.createMediaElementSource).toHaveBeenCalledTimes(1); + expect(mock.mediaElementSourceNode.disconnect).toHaveBeenCalledTimes(1); + }); + + it("keeps returning the cached node when a reused element's src stays eligible", async () => { + const { transport, mock, gen: gen1 } = setupTransport(100); + const el = document.createElement("audio"); + el.setAttribute("src", "/assets/vo.mp3"); + + await transport.scheduleMediaElementPlayback(el, 0, 0, 0, 1, gen1, 1); + transport.stopAll(); + mock.mediaElementSourceNode.disconnect.mockClear(); + el.setAttribute("src", "/assets/other-same-origin-clip.mp3"); + const gen2 = transport.startGeneration(); + const second = await transport.scheduleMediaElementPlayback(el, 0, 0, 0, 1, gen2, 1); + + expect(second).not.toBeNull(); + expect(mock.ctx.createMediaElementSource).toHaveBeenCalledTimes(1); + expect(mock.mediaElementSourceNode.disconnect).not.toHaveBeenCalled(); + }); }); it("tracks play generation for async race prevention", () => { diff --git a/packages/core/src/runtime/webAudioTransport.ts b/packages/core/src/runtime/webAudioTransport.ts index 418691d8d..2761c9854 100644 --- a/packages/core/src/runtime/webAudioTransport.ts +++ b/packages/core/src/runtime/webAudioTransport.ts @@ -255,13 +255,47 @@ export class WebAudioTransport { * element away from its native output, so the question has to be settled * before it, and there is no undo afterwards. * - * `init.ts` routes on the same verdict before ever calling in; this stays the - * enforcement point so a direct caller (studio, player) cannot reopen the - * one-way door. + * `init.ts` routes on the same verdict before ever calling in, and + * `AudioRow.tsx`'s standalone preview player classifies over its own + * throwaway `AudioContext` before its own `createMediaElementSource` call — + * this method is A enforcement point, not THE enforcement point; every + * caller that can reach `createMediaElementSource` is expected to classify + * first. What this method DOES own is the cache below: `_mediaElementSources` + * is keyed by element identity, not by asset, so a cache hit alone says + * nothing about the element's CURRENT resource. Reclassifying on every call + * — cache hit included — means a `src` mutation an outer caller missed (a + * pooled element swapped from a same-origin clip to a cross-origin one + * without going through a fresh generation) can't leave a stale + * `web-audio` verdict silently attached to the new resource. */ private acquireMediaElementSource(el: HTMLMediaElement): MediaElementAudioSourceNode | null { const cached = this._mediaElementSources.get(el); - if (cached) return cached; + if (cached) { + // The node itself doesn't change identity on a src swap, but its + // eligibility can: the Web Audio spec's tainted-origin check runs + // against the element's CURRENT underlying resource, not the one that + // was current when the node was built. A same-origin-to-cross-origin + // mutation on this element would otherwise keep returning the old + // (now-silent) node forever — `destroy()` was the only thing that ever + // cleared this cache, so a long-lived element that changed sources + // stayed silenced for the rest of the session (the R2 finding this + // block exists to close). The node is still a one-way door — it can't + // be un-created, and the element's native output is gone either way — + // so disconnecting it just stops it feeding a graph that no longer + // matches the asset; the caller falls back to the decode-only path. + const route = classifyWebAudioMediaRoute(el); + if (route.kind !== "web-audio") { + try { + cached.disconnect(); + } catch { + // Already torn down. + } + this._mediaElementSources.delete(el); + reportWebAudioMediaRoute(el, route); + return null; + } + return cached; + } if (!this._ctx) return null; const route = classifyWebAudioMediaRoute(el); if (route.kind !== "web-audio") {