Skip to content

Commit 3252011

Browse files
author
t
committed
Scope the transformed-source cache per handler (fix cross-handler bleed)
The TS-strip + elision cache was module-global, keyed only on (path, mtime), but the cached bytes bake in a handler's elision verdict. Two createRequestHandler instances for the same app with different elision settings (a multi-tenant embedder, or the new differential elision test) therefore poisoned each other: booting an ON handler then an OFF handler in one process made the OFF handler serve the ON handler's already-elided source for the same path, silently over-stripping on the OFF side. Move the cache into per-handler state (state.tsCache). Single-handler behaviour (production) is unchanged; multiple handlers now each serve their own verdict. Regression test boots ON-then-OFF and asserts the OFF handler keeps the elided import the ON handler strips. Found in review of the differential elision test (#181).
1 parent 3817eef commit 3252011

2 files changed

Lines changed: 52 additions & 11 deletions

File tree

‎packages/server/src/dev.js‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,13 @@ const MIME = {
105105
* lint rules catch these at edit time. webjs is buildless end-to-end:
106106
* there is no bundler fallback.
107107
*
108-
* @type {Map<string, { mtimeMs: number, code: string, map: string | null }>}
108+
* The transformed bytes are cached per request handler in `state.tsCache`
109+
* (a `Map<string, { mtimeMs, code, map }>`), bounded to `TS_CACHE_MAX`
110+
* entries. The cache is per-handler rather than module-global because the
111+
* cached code bakes in that handler's elision verdict, so two handlers for
112+
* the same app with different elision settings must not share it.
109113
*/
110114
const TS_CACHE_MAX = 500;
111-
const TS_CACHE = new Map();
112115

113116
/**
114117
* Auto-load `<appDir>/.env` into `process.env` once at boot. Mirrors
@@ -291,6 +294,12 @@ export async function createRequestHandler(opts) {
291294
elidableComponents: new Set(),
292295
inertRouteModules: new Set(),
293296
browserBoundFiles: null,
297+
// Transformed-source cache (stripped TS + applied elision). Per-handler,
298+
// NOT module-global: the cached bytes bake in THIS handler's elision
299+
// verdict, so two handlers for the same app with different elision
300+
// settings (a multi-tenant embedder, or the differential elision test)
301+
// must not share it, or the second would serve the first's elided source.
302+
tsCache: new Map(),
294303
};
295304

296305
// All whole-app analysis is built lazily on the first request, memoized so
@@ -524,12 +533,12 @@ export async function createRequestHandler(opts) {
524533
// it so routing reflects added/removed route files immediately.
525534
state.routeTable = await buildRouteTable(appDir);
526535
clearVendorCache();
527-
TS_CACHE.clear();
536+
state.tsCache.clear();
528537
// Invalidate the lazy analysis; the next request rebuilds the graph,
529538
// component scan, gate, action index, middleware, elision, and vendor map.
530539
// Wait out any in-flight build first so it cannot commit stale results
531540
// after the reset. A dependency edit can flip an elision verdict without
532-
// changing an importer's mtime, hence the TS_CACHE.clear above.
541+
// changing an importer's mtime, hence the state.tsCache.clear above.
533542
if (readyInFlight) { try { await readyInFlight; } catch {} }
534543
// Bump the vendor generation so a vendor resolve still in flight from the
535544
// previous build cannot flip vendorResolved against the fresh state.
@@ -1052,7 +1061,7 @@ async function handleCore(req, ctx) {
10521061
appDir,
10531062
};
10541063
if (/\.m?ts$/.test(abs)) {
1055-
return tsResponse(abs, dev, elideOpts);
1064+
return tsResponse(abs, dev, elideOpts, state.tsCache);
10561065
}
10571066
if (/\.m?js$/.test(abs)) {
10581067
return jsModuleResponse(abs, dev, elideOpts);
@@ -1444,9 +1453,9 @@ async function stripTs(source, _abs) {
14441453
* @param {boolean} dev
14451454
* @param {{ moduleGraph: any, elidableComponents: Set<string>|undefined, appDir: string }} [elideOpts]
14461455
*/
1447-
async function tsResponse(abs, dev, elideOpts) {
1456+
async function tsResponse(abs, dev, elideOpts, cache) {
14481457
const st = await stat(abs);
1449-
const cached = TS_CACHE.get(abs);
1458+
const cached = cache.get(abs);
14501459
if (cached && cached.mtimeMs === st.mtimeMs) {
14511460
return new Response(cached.code, {
14521461
headers: {
@@ -1499,11 +1508,11 @@ async function tsResponse(abs, dev, elideOpts) {
14991508
);
15001509
}
15011510
// Evict oldest entry if cache is full (simple FIFO: Map preserves insertion order).
1502-
if (TS_CACHE.size >= TS_CACHE_MAX) {
1503-
const oldest = TS_CACHE.keys().next().value;
1504-
TS_CACHE.delete(oldest);
1511+
if (cache.size >= TS_CACHE_MAX) {
1512+
const oldest = cache.keys().next().value;
1513+
cache.delete(oldest);
15051514
}
1506-
TS_CACHE.set(abs, { mtimeMs: st.mtimeMs, code, map: null });
1515+
cache.set(abs, { mtimeMs: st.mtimeMs, code, map: null });
15071516
return new Response(code, {
15081517
headers: {
15091518
'content-type': 'application/javascript; charset=utf-8',

‎packages/server/test/elision/differential-elision.test.js‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,38 @@ test('the mixed page actually elides JS on the ON side (the diff is not vacuous)
130130
);
131131
});
132132

133+
test('served module source reflects each handler\'s own elision verdict (no cross-handler cache bleed)', async () => {
134+
// Regression: the transformed-source cache used to be module-global keyed
135+
// on (path, mtime), but the cached bytes bake in a handler's elision
136+
// verdict. Booting an ON handler then an OFF handler in one process made
137+
// the OFF handler serve the ON handler's already-elided source for the
138+
// same path. A multi-tenant embedder running createRequestHandler per app
139+
// with different elision settings hit the same poisoning. The cache is now
140+
// per-handler (state.tsCache). ON is warmed FIRST here so a shared cache
141+
// would be poisoned by the time OFF reads it.
142+
const ORIG = process.env.WEBJS_ELIDE;
143+
const IMPORT = /import\s+['"][^'"]*build-stamp\.ts['"]/;
144+
try {
145+
delete process.env.WEBJS_ELIDE;
146+
const hOn = await createRequestHandler({ appDir: BLOG, dev: false });
147+
if (hOn.warmup) await hOn.warmup();
148+
const onSrc = await (await hOn.handle(new Request('http://localhost/app/page.ts'))).text();
149+
150+
process.env.WEBJS_ELIDE = '0';
151+
const hOff = await createRequestHandler({ appDir: BLOG, dev: false });
152+
if (hOff.warmup) await hOff.warmup();
153+
const offSrc = await (await hOff.handle(new Request('http://localhost/app/page.ts'))).text();
154+
155+
// The page side-effect-imports the display-only <build-stamp>. ON strips
156+
// that import (the module is never downloaded); OFF keeps it.
157+
assert.ok(!IMPORT.test(onSrc), 'ON handler must strip the elided build-stamp import from the served page source');
158+
assert.ok(IMPORT.test(offSrc), 'OFF handler must keep the build-stamp import (cross-handler cache bleed would strip it)');
159+
} finally {
160+
if (ORIG === undefined) delete process.env.WEBJS_ELIDE;
161+
else process.env.WEBJS_ELIDE = ORIG;
162+
}
163+
});
164+
133165
test('counterfactual: the masked diff is sensitive to a real body change', () => {
134166
// The comparator must not pass vacuously by masking too much. A dropped
135167
// NEEDED module manifests, in the worst case, as missing rendered output;

0 commit comments

Comments
 (0)