Skip to content

Commit 3cdfe7e

Browse files
authored
fix(runtime): lazy-loaded globals should shadow on inherited [[Set]] (#34405)
## Summary Fixes #34403. Lazy-loaded global accessors (`Response`, `Request`, `ReadableStream`, `WebSocket`, `CompressionStream`, ...) installed via `propNonEnumerableLazyLoaded` / `propWritableLazyLoaded` stored assigned values in a shared closure and ignored the receiver. Assigning through an object whose prototype chain includes `globalThis` (the `whatwg-fetch` / `cross-fetch` polyfill idiom) clobbered the global instead of creating a shadowing own property on the receiver, breaking `x instanceof Response` for real fetch responses and tripping up wasm-bindgen loaders such as `@swc/wasm-web`. The setter now uses `Object.defineProperty(this, name, ...)` to define an own data property on the receiver, matching the pre-2.8 data-property `[[Set]]` semantics. A new `core.defineGlobalProperties` helper stamps the property name onto each lazy descriptor at install time so the setter knows which name to define. After the first set, the accessor is replaced by a regular data property on whichever object received the set, so subsequent reads on that object skip the lazy loader entirely: - Direct assignment to `globalThis`: descriptor becomes a data descriptor with the assigned value (same as 2.7.x). - Assignment via an inheriting object: creates a shadowing own property on the inheritor; `globalThis` is untouched. ## Test plan - [x] New spec test `tests/specs/run/lazy_global_polyfill_shadowing` covers the polyfill pattern, the `instanceof` invariant, and direct `globalThis` assignment. - [x] `cargo test --test specs -- lazy_global` passes. - [x] Related spec tests (`fetch`, `window`) still pass.
1 parent f202457 commit 3cdfe7e

7 files changed

Lines changed: 166 additions & 32 deletions

File tree

libs/core/01_core.js

Lines changed: 58 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
ErrorCaptureStackTrace,
1010
FunctionPrototypeBind,
1111
ObjectAssign,
12+
ObjectDefineProperties,
13+
ObjectDefineProperty,
1214
ObjectFreeze,
1315
ObjectFromEntries,
1416
ObjectKeys,
@@ -842,50 +844,79 @@
842844
};
843845
}
844846

845-
function propWritableLazyLoaded(getter, loadFn) {
846-
let valueIsSet = false;
847-
let value;
847+
// Marker symbol identifying a lazy-loaded property descriptor. The property
848+
// name is filled in by `defineGlobalProperties` at install time so the
849+
// setter can mimic data-property [[Set]] semantics (define an own data
850+
// property on the receiver) instead of mutating a closure shared across all
851+
// receivers - see denoland/deno#34403.
852+
const lazyNameSym = Symbol("lazyName");
848853

849-
return {
854+
function propWritableLazyLoaded(getter, loadFn) {
855+
const desc = {
850856
get() {
851-
const loadedValue = loadFn();
852-
if (valueIsSet) {
853-
return value;
854-
} else {
855-
return getter(loadedValue);
856-
}
857+
return getter(loadFn());
857858
},
858859
set(v) {
859-
loadFn();
860-
valueIsSet = true;
861-
value = v;
860+
ObjectDefineProperty(this, desc[lazyNameSym], {
861+
value: v,
862+
writable: true,
863+
enumerable: true,
864+
configurable: true,
865+
});
862866
},
863867
enumerable: true,
864868
configurable: true,
869+
[lazyNameSym]: undefined,
865870
};
871+
return desc;
866872
}
867873

868874
function propNonEnumerableLazyLoaded(getter, loadFn) {
869-
let valueIsSet = false;
870-
let value;
871-
872-
return {
875+
const desc = {
873876
get() {
874-
const loadedValue = loadFn();
875-
if (valueIsSet) {
876-
return value;
877-
} else {
878-
return getter(loadedValue);
879-
}
877+
return getter(loadFn());
880878
},
881879
set(v) {
882-
loadFn();
883-
valueIsSet = true;
884-
value = v;
880+
const name = desc[lazyNameSym];
881+
// Match OrdinarySetWithOwnDescriptor semantics for an inherited
882+
// writable data property: a direct set on the holder updates the
883+
// value in place (preserving enumerable: false), while an inherited
884+
// set creates a new enumerable own data property on the receiver.
885+
if (ObjectHasOwn(this, name)) {
886+
ObjectDefineProperty(this, name, {
887+
value: v,
888+
writable: true,
889+
enumerable: false,
890+
configurable: true,
891+
});
892+
} else {
893+
ObjectDefineProperty(this, name, {
894+
value: v,
895+
writable: true,
896+
enumerable: true,
897+
configurable: true,
898+
});
899+
}
885900
},
886901
enumerable: false,
887902
configurable: true,
903+
[lazyNameSym]: undefined,
888904
};
905+
return desc;
906+
}
907+
908+
// Like `Object.defineProperties`, but also stamps the property name into any
909+
// lazy-loaded descriptors so their setters can target the correct receiver.
910+
function defineGlobalProperties(target, props) {
911+
const keys = ObjectKeys(props);
912+
for (let i = 0; i < keys.length; i++) {
913+
const key = keys[i];
914+
const desc = props[key];
915+
if (desc !== null && typeof desc === "object" && lazyNameSym in desc) {
916+
desc[lazyNameSym] = key;
917+
}
918+
}
919+
ObjectDefineProperties(target, props);
889920
}
890921

891922
function createLazyLoader(specifier) {
@@ -1199,6 +1230,7 @@
11991230
propGetterOnly,
12001231
propWritableLazyLoaded,
12011232
propNonEnumerableLazyLoaded,
1233+
defineGlobalProperties,
12021234
createLazyLoader,
12031235
loadExtScript,
12041236
createCancelHandle: () => op_cancel_handle(),

libs/core/core.d.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,11 @@ export namespace core {
377377
loadFn: LazyLoader<T>,
378378
): PropertyDescriptor;
379379

380+
function defineGlobalProperties(
381+
target: object,
382+
props: PropertyDescriptorMap,
383+
): void;
384+
380385
type LazyLoader<T> = () => T;
381386
function createLazyLoader<T = unknown>(specifier: string): LazyLoader<T>;
382387

runtime/js/90_deno_ns.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ import { unstableIds } from "ext:deno_features/flags.js";
6262
const { loadWebGPU } = core.loadExtScript("ext:deno_webgpu/00_init.js");
6363
import { bundle } from "ext:deno_bundle_runtime/bundle.ts";
6464

65-
const { ObjectDefineProperties, ObjectDefineProperty, Float64Array } =
66-
primordials;
65+
const { ObjectDefineProperty, Float64Array } = primordials;
6766

6867
const loadQuic = core.createLazyLoader("ext:deno_net/03_quic.js");
6968
const loadWebTransport = core.createLazyLoader(
@@ -314,7 +313,7 @@ denoNsUnstableById[unstableIds.net] = {
314313
),
315314
};
316315

317-
ObjectDefineProperties(denoNsUnstableById[unstableIds.net], {
316+
core.defineGlobalProperties(denoNsUnstableById[unstableIds.net], {
318317
connectQuic: core.propWritableLazyLoaded((q) => q.connectQuic, loadQuic),
319318
QuicEndpoint: core.propWritableLazyLoaded((q) => q.QuicEndpoint, loadQuic),
320319
QuicBidirectionalStream: core.propWritableLazyLoaded(
@@ -343,7 +342,7 @@ ObjectDefineProperties(denoNsUnstableById[unstableIds.net], {
343342
denoNsUnstableById[unstableIds.webgpu] = {
344343
UnsafeWindowSurface: surface.UnsafeWindowSurface,
345344
};
346-
ObjectDefineProperties(denoNsUnstableById[unstableIds.webgpu], {
345+
core.defineGlobalProperties(denoNsUnstableById[unstableIds.webgpu], {
347346
webgpu: core.propWritableLazyLoaded(
348347
(webgpu) => webgpu.denoNsWebGPU,
349348
loadWebGPU,

runtime/js/99_main.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,7 @@ function dispatchUnloadEvent() {
548548

549549
let hasBootstrapped = false;
550550
// Set up global properties shared by main and worker runtime.
551-
ObjectDefineProperties(globalThis, windowOrWorkerGlobalScope);
551+
core.defineGlobalProperties(globalThis, windowOrWorkerGlobalScope);
552552

553553
// Set up global properties shared by main and worker runtime that are exposed
554554
// by unstable features if those are enabled.
@@ -564,7 +564,7 @@ function exposeUnstableFeaturesForWindowOrWorkerGlobalScope(unstableFeatures) {
564564
const featureId = featureIds[i];
565565
if (ArrayPrototypeIncludes(unstableFeatures, featureId)) {
566566
const props = unstableForWindowOrWorkerGlobalScope[featureId];
567-
ObjectDefineProperties(globalThis, { ...props });
567+
core.defineGlobalProperties(globalThis, { ...props });
568568
}
569569
}
570570
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"tests": {
3+
"polyfill_shadow": {
4+
"args": "run main.js",
5+
"output": "main.out"
6+
}
7+
}
8+
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Regression test for https://github.com/denoland/deno/issues/34403
2+
// Lazy-loaded globals (Response, Request, ReadableStream, WebSocket, ...) used
3+
// to be writable data properties. After they were converted to accessor
4+
// properties for lazy loading, assigning through an object whose prototype is
5+
// globalThis (the cross-fetch / whatwg-fetch polyfill pattern) would clobber
6+
// the global instead of shadowing on the receiver.
7+
8+
const originalResponse = globalThis.Response;
9+
const originalRequest = globalThis.Request;
10+
const originalReadableStream = globalThis.ReadableStream;
11+
const originalWebSocket = globalThis.WebSocket;
12+
13+
// Mimic the cross-fetch / whatwg-fetch polyfill pattern.
14+
function Self() {}
15+
Self.prototype = globalThis;
16+
const g = new Self();
17+
g.Response = class PolyfillResponse {};
18+
g.Request = class PolyfillRequest {};
19+
g.ReadableStream = class PolyfillReadableStream {};
20+
g.WebSocket = class PolyfillWebSocket {};
21+
22+
console.log(
23+
"global Response untouched:",
24+
globalThis.Response === originalResponse,
25+
);
26+
console.log(
27+
"global Request untouched:",
28+
globalThis.Request === originalRequest,
29+
);
30+
console.log(
31+
"global ReadableStream untouched:",
32+
globalThis.ReadableStream === originalReadableStream,
33+
);
34+
console.log(
35+
"global WebSocket untouched:",
36+
globalThis.WebSocket === originalWebSocket,
37+
);
38+
39+
console.log(
40+
"own Response on receiver:",
41+
Object.prototype.hasOwnProperty.call(g, "Response"),
42+
);
43+
console.log(
44+
"own Request on receiver:",
45+
Object.prototype.hasOwnProperty.call(g, "Request"),
46+
);
47+
48+
// The shadowing own property on the inheriting receiver should be a normal
49+
// enumerable, writable, configurable data property - matching standard
50+
// [[Set]] semantics for an inherited writable data property.
51+
const receiverDesc = Object.getOwnPropertyDescriptor(g, "Response");
52+
console.log(
53+
"receiver Response descriptor:",
54+
JSON.stringify({
55+
hasValue: Object.hasOwn(receiverDesc, "value"),
56+
writable: receiverDesc.writable,
57+
enumerable: receiverDesc.enumerable,
58+
configurable: receiverDesc.configurable,
59+
}),
60+
);
61+
62+
// `new Response()` still produces a real Response (instanceof works).
63+
const r = new Response("hello");
64+
console.log("real Response instanceof Response:", r instanceof Response);
65+
66+
// Direct assignment to globalThis still works, and preserves the original
67+
// non-enumerable attribute of the global descriptor.
68+
globalThis.WebSocket = "patched";
69+
console.log("direct set on globalThis:", globalThis.WebSocket === "patched");
70+
const globalDesc = Object.getOwnPropertyDescriptor(globalThis, "WebSocket");
71+
console.log(
72+
"global WebSocket descriptor:",
73+
JSON.stringify({
74+
hasValue: Object.hasOwn(globalDesc, "value"),
75+
writable: globalDesc.writable,
76+
enumerable: globalDesc.enumerable,
77+
configurable: globalDesc.configurable,
78+
}),
79+
);
80+
globalThis.WebSocket = originalWebSocket;
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
global Response untouched: true
2+
global Request untouched: true
3+
global ReadableStream untouched: true
4+
global WebSocket untouched: true
5+
own Response on receiver: true
6+
own Request on receiver: true
7+
receiver Response descriptor: {"hasValue":true,"writable":true,"enumerable":true,"configurable":true}
8+
real Response instanceof Response: true
9+
direct set on globalThis: true
10+
global WebSocket descriptor: {"hasValue":true,"writable":true,"enumerable":false,"configurable":true}

0 commit comments

Comments
 (0)