Skip to content

Commit b3ee203

Browse files
Api manipulation method values (#2703)
* Support value descriptors in apiManipulation * Refine apiManipulation value descriptor handling * Trim apiManipulation descriptor duplication * Test value descriptor override without define: true Adds two unit tests for apiManipulation: - Confirms an existing DOM-style value function descriptor (set via Object.defineProperty with writable/configurable/enumerable) can be remotely overridden via a value descriptor change *without* define: true, and that the original descriptor attributes (writable/configurable/enumerable) are preserved through the merge in wrapProperty(). - Confirms that setting define: true on a change for a property that already exists still falls through to wrapProperty(), so the new function replaces the existing one rather than being skipped. Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com> * Mask method replacements to preserve native identity When apiManipulation replaces an existing method-valued descriptor with a configured function, mask the replacement against the original DOM method so that toString(), toString.toString(), .name and .length still resemble the native method. Without this, third-party scripts can trivially detect the override via fn.toString() ('function noop() {}') or fn.name. Implementation: in wrapApiDescriptor, when the new descriptor's value is a function and an existing own value descriptor with a function value exists, wrap the configured replacement with maskMethodReplacement(). That helper creates a fresh wrapper function (so we never mutate shared singletons such as functionMap.noop), copies the original function's name and length onto it, and applies wrapToString() against the original to preserve toString output. The resulting value is then defined via ContentFeature.defineProperty in the normal way, which adds its debug flag and a second wrapToString layer; the two layers compose correctly because the inner mask is wrapToString rather than wrapFunction (the latter creates a non-configurable own toString that violates the Proxy get-trap invariant once the outer wrapToString is added on top). Adds unit tests asserting: - toString() returns the original method's source rather than the replacement's body - toString.toString() still resolves to Function.prototype.toString - .name and .length match the original method - When the original is a real native function (Object.prototype .hasOwnProperty), the masked toString() contains '[native code]' - The configured replacement is still what executes at call time Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com> * Use captured globals in apiManipulation method masking The masking helper added in the previous commit invoked the configured replacement via the page-visible `Reflect.apply` and set `name`/`length` on the wrapper via the page-visible `Object.defineProperty`. Both are re-readable by hostile pages after content scope initialisation: - `Reflect.apply` is reassignable on `globalThis.Reflect`, letting a page observe or alter every invocation of the protected method. - `Object.defineProperty` is reassignable on `globalThis.Object`, letting a page block or poison the name/length masking. Switch the helper to: - `ReflectApply` — new bound primitive exported from captured-globals, matching the existing `ReflectDeleteProperty` pattern. - `objectDefineProperty` — already-exported captured `Object.defineProperty`. Adds a unit test that swaps in spies for `globalThis.Reflect.apply` and `globalThis.Object.defineProperty` after feature construction and asserts: 1. `Object.defineProperty` is not called during `wrapApiDescriptor` (the helper used captured `objectDefineProperty`). 2. Masked `name`/`length`/`toString()` remain intact under tampering. 3. End-to-end invocation still routes to the configured replacement. Note: `ContentFeature.defineProperty`'s outer debug-wrapper still reads `Reflect.apply` from the page; that is pre-existing and out of scope for this helper. The captured `ReflectApply` here guarantees the helper's own inner call (helper → configured replacement) cannot be intercepted. Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com> * apiManipulation: shadow-define inherited methods + setterValue support Adds two capabilities to the descriptor-change path so configs can target real-world media API surfaces that the existing wrapApiDescriptor cannot: 1. Shadow-define inherited methods. The previous define: true guard was !(key in api), which is alse for any property reachable via the prototype chain (e.g. MediaDevices.prototype.addEventListener, inherited from EventTarget.prototype). Switch the guard to !Object.prototype.hasOwnProperty.call(api, key) so a config can define an own property on MediaDevices.prototype that shadows the inherited one without modifying EventTarget.prototype (which would affect every other EventTarget consumer - window, document, elements, etc.). 2. setterValue field on descriptor changes. Accessor properties such as MediaDevices.prototype.ondevicechange (an EventHandler IDL attribute) are a getter+setter pair. The existing getterValue path produces only { get }, so wrapProperty's {...origDescriptor, ...descriptor} merge preserves the original setter; assigning mediaDevices.ondevicechange = fn still registers a real listener. Adding setterValue to the descriptor shape lets configs override the setter half too, invoking the configured function via captured ReflectApply on each assignment. getterValue and setterValue may be supplied independently or together. Validation is updated to accept the accessor shape when only setterValue is present, and to reject mixing accessor (getterValue/setterValue) with value (�alue) shapes in a single change. Adds unit tests covering: - shadow-define an inherited method with define: true - no-op behaviour when define: true is omitted and key is inherited - setter override alongside getter - setter-only override (original getter preserved) - validation for setterValue-only changes - validation rejecting setterValue + value mixing Motivation: the original Claude camera-prompt-on-login report turned out to be triggered by Claude subscribing to mediaDevices.devicechange during its bootstrap (via both �ddEventListener('devicechange', ...) and mediaDevices.ondevicechange = ...). With these two additions a single apiManipulation config can suppress every JS-side device-enumeration trigger without touching EventTarget.prototype or shipping a separate JS shim. * Mask setterValue replacement against original native setter When apiManipulation overrides a setter (`setterValue`) on an existing accessor descriptor, the replacement setter is now masked against the original `origDescriptor.set` via the existing `maskMethodReplacement` helper. This preserves the observable identity of the accessor for descriptor inspection: `Object.getOwnPropertyDescriptor(...).set.toString()`, `.set.name`, and `.set.length` mirror the original setter rather than exposing our internal `setter(v) {...}` shape. For native event-handler IDL attributes (e.g. `Element.prototype.onclick`, `MediaDevices.prototype.ondevicechange`) this prevents trivial detection of the override via descriptor probing. Implementation: extract `origDescriptor = getOwnPropertyDescriptor(api, key)` once per call in `wrapApiDescriptor`, then branch on `descriptorKind`: - For value descriptors, mask the replacement against the original function value (existing behaviour, refactored to share the lookup). - For accessor descriptors, additionally mask `descriptor.set` against `origDescriptor.set` whenever both are functions. The getter path is left alone because configured getters are generated by processAttr per-read and don't typically need toString-fidelity. Also fixes blocking TypeScript errors from the previous commit: - `createApiDescriptor` now uses a permissive intermediate object type internally so `descriptor.get` / `descriptor.set` assignments inside the accessor branch type-check, and returns a cast back to `Partial<StrictPropertyDescriptor>`. - The `configSetting` parameter type now allows `undefined` (matches the setter-only call shape), with a localised cast inside the value branch where `wrapApiDescriptor`'s early-return already guarantees a defined value. Adds a unit test asserting that, after applying a `setterValue` change over an existing accessor descriptor, the wrapped `descriptor.set`'s `toString()`, `toString.toString()`, `.name`, and `.length` match the original setter, and the configured replacement still swallows the assignment (original setter not invoked). Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com> * apiManipulation: define only for missing APIs; wrap inherited methods Retain define:true for properties absent from the prototype chain only. When an API exists (own or inherited), apply value/getterValue/setterValue overrides—including shadow-defining inherited methods such as MediaDevices.prototype.addEventListener without requiring define. * Revert define behavior and JSDoc to branch baseline Restore unchanged define semantics from c7be658. Value/getterValue/setterValue and method masking remain; inherited APIs still require define: true to shadow-define. * Restore define JSDoc and main semantics (key in api) Revert expanded define documentation and hasOwnProperty shadow-define behavior. Remove inherited shadow-define tests. define:true remains for adding properties that are absent from the target object. * Integrate privacy-configuration #5215 MediaDevices apiManipulation config Bump @duckduckgo/privacy-configuration to 1779290401870 (merged PR #5215 schema: setterValue/value descriptors with original define comment). Restore inherited-property shadow-defining without define: true so MediaDevices.prototype.addEventListener/removeEventListener overrides match the deployed remote-config shape. Add unit coverage for the #5215 apiChanges payload and schema validation. * apiManipulation: guard inherited descriptor merges against invalid shapes Extract mergePropertyDescriptors from wrapProperty so shadow-defining inherited APIs uses the same value/accessor compatibility rules. Skip the override (instead of throwing) when remote config shape disagrees with the live prototype descriptor kind. Clarify define vs inherited shadow-define JSDoc. Add unit coverage for descriptor-kind mismatches on inherited properties. * apiManipulation: require define:true for inherited shadow-define Remove the implicit else branch that shadow-defined prototype-chain properties without define. Inherited overrides now use the same define:true && !hasOwnProperty path as intended, with descriptor-kind guards via mergePropertyDescriptors. Update unit tests; privacy-configuration #5215 entries will need define: true added in remote config. * Fix mergePropertyDescriptors return type for StrictPropertyDescriptor Normalize merged descriptors so tsc accepts them at defineProperty and wrapProperty call sites. No remote config change required. * Fix tab-suspension test config for updated tabSuspension schema privacy-configuration #1779290401870 expects inputFieldFocusDetection settings as { state } object, not a bare string. * apiManipulation: restore implicit inherited shadow-define Re-add the else path for prototype-chain properties so remote config (e.g. MediaDevices addEventListener) works without define: true. Properties absent from the entire chain still no-op unless define: true. Descriptor-kind mismatches still skip via mergePropertyDescriptors. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
1 parent c5601c2 commit b3ee203

8 files changed

Lines changed: 767 additions & 40 deletions

File tree

injected/integration-test/test-pages/tab-suspension/config/tab-suspension.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
"state": "enabled",
77
"exceptions": [],
88
"settings": {
9-
"inputFieldFocusDetection": "enabled"
9+
"inputFieldFocusDetection": {
10+
"state": "enabled"
11+
}
1012
}
1113
}
1214
},

injected/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
},
3636
"type": "module",
3737
"dependencies": {
38-
"@duckduckgo/privacy-configuration": "github:duckduckgo/privacy-configuration#1772634901001",
38+
"@duckduckgo/privacy-configuration": "github:duckduckgo/privacy-configuration#1779290401870",
3939
"@duckduckgo/tracker-surrogates": "github:duckduckgo/tracker-surrogates#0528e3226df15b1a3e319ad68ef76612a8f26623",
4040
"esbuild": "^0.27.4",
4141
"minimist": "^1.2.8",

injected/src/captured-globals.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export const JSONstringify = JSON.stringify;
3838
export const JSONparse = JSON.parse;
3939
export const Arrayfrom = Array.from;
4040
export const ReflectDeleteProperty = Reflect.deleteProperty.bind(Reflect);
41+
export const ReflectApply = Reflect.apply.bind(Reflect);
4142
export const getRandomValues = globalThis.crypto?.getRandomValues?.bind(globalThis.crypto);
4243

4344
// Secure context only - crypto.subtle is unavailable on HTTP

injected/src/features/api-manipulation.js

Lines changed: 186 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
/**
22
* This feature allows remote configuration of APIs that exist within the DOM.
3-
* We support removal of APIs and returning different values from getters.
3+
* We support removal of APIs, returning different values from getters, and
4+
* replacing value-based properties such as methods.
45
*
56
* @module API manipulation
67
*/
78
import ContentFeature from '../content-feature.js';
89
// eslint-disable-next-line no-redeclare
9-
import { hasOwnProperty } from '../captured-globals.js';
10+
import { ReflectApply, getOwnPropertyDescriptor, hasOwnProperty, objectDefineProperty } from '../captured-globals.js';
1011
import { processAttr } from '../utils.js';
12+
import { mergePropertyDescriptors, wrapToString } from '../wrapper-utils.js';
1113

1214
/**
1315
* @internal
@@ -45,16 +47,23 @@ export default class ApiManipulation extends ContentFeature {
4547
return true;
4648
}
4749
if (change.type === 'descriptor') {
48-
if (change.enumerable && typeof change.enumerable !== 'boolean') {
50+
if ('enumerable' in change && typeof change.enumerable !== 'boolean') {
4951
return false;
5052
}
51-
if (change.configurable && typeof change.configurable !== 'boolean') {
53+
if ('configurable' in change && typeof change.configurable !== 'boolean') {
5254
return false;
5355
}
5456
if ('define' in change && typeof change.define !== 'boolean') {
5557
return false;
5658
}
57-
return typeof change.getterValue !== 'undefined';
59+
const hasGetterValue = typeof change.getterValue !== 'undefined';
60+
const hasSetterValue = typeof change.setterValue !== 'undefined';
61+
const hasValue = typeof change.value !== 'undefined';
62+
// Either a value descriptor (with `value`) or an accessor descriptor (with `getterValue`
63+
// and/or `setterValue`) - the two shapes are mutually exclusive.
64+
const isAccessorShape = hasGetterValue || hasSetterValue;
65+
const isValueShape = hasValue;
66+
return isAccessorShape !== isValueShape;
5867
}
5968
return false;
6069
}
@@ -66,9 +75,11 @@ export default class ApiManipulation extends ContentFeature {
6675
* @typedef {Object} APIChange
6776
* @property {"remove"|"descriptor"} type
6877
* @property {import('../utils.js').ConfigSetting} [getterValue] - The value returned from a getter.
78+
* @property {import('../utils.js').ConfigSetting} [setterValue] - The function invoked when the property is assigned. Used alongside (or instead of) getterValue to override accessor-style properties such as event handlers (e.g., `MediaDevices.prototype.ondevicechange`).
79+
* @property {import('../utils.js').ConfigSetting} [value] - The value assigned to a value descriptor, including methods.
6980
* @property {boolean} [enumerable] - Whether the property is enumerable.
7081
* @property {boolean} [configurable] - Whether the property is configurable.
71-
* @property {boolean} [define] - Whether to define the property if it does not exist.
82+
* @property {boolean} [define] - When true, define a new own property if the key is absent from `api` and its entire prototype chain. When false (default), skip changes for properties that do not exist at all; override own properties via `wrapProperty`; override inherited properties by shadow-defining an own property on `api`.
7283
*/
7384

7485
/**
@@ -111,29 +122,180 @@ export default class ApiManipulation extends ContentFeature {
111122
*/
112123
wrapApiDescriptor(api, key, change) {
113124
const getterValue = change.getterValue;
114-
if (getterValue) {
115-
const descriptor = {
116-
get: () => processAttr(getterValue, undefined),
117-
};
118-
if ('enumerable' in change) {
119-
descriptor.enumerable = change.enumerable;
125+
const setterValue = change.setterValue;
126+
const value = change.value;
127+
const descriptorKind =
128+
getterValue !== undefined || setterValue !== undefined ? 'getter' : value !== undefined ? 'value' : undefined;
129+
const configSetting = descriptorKind === 'getter' ? getterValue : value;
130+
// `setterValue` may be supplied on its own (override only the setter, keep the original
131+
// getter); in that case configSetting (getterValue) is undefined and createApiDescriptor
132+
// skips the `get` half.
133+
if (!descriptorKind || (descriptorKind === 'value' && configSetting === undefined)) {
134+
return;
135+
}
136+
const descriptor = this.createApiDescriptor(descriptorKind, configSetting, change);
137+
const origDescriptor = this.findPropertyDescriptor(api, key);
138+
if (!origDescriptor) {
139+
if (change.define === true) {
140+
this.defineProperty(api, key, this.createDefineDescriptor(descriptor, descriptorKind));
120141
}
121-
if ('configurable' in change) {
122-
descriptor.configurable = change.configurable;
142+
return;
143+
}
144+
// When replacing an existing method-valued descriptor, mask the replacement
145+
// against the original method so `toString()`, `toString.toString()`, `.name`
146+
// and `.length` continue to resemble the native method. Without this, sites can
147+
// trivially detect the override via `fn.toString()` / `fn.name`.
148+
if (descriptorKind === 'value') {
149+
const valueDescriptor = /** @type {{ value?: any }} */ (descriptor);
150+
if (typeof valueDescriptor.value === 'function' && typeof origDescriptor.value === 'function') {
151+
valueDescriptor.value = this.maskMethodReplacement(valueDescriptor.value, origDescriptor.value);
123152
}
124-
// If 'define' is true and property does not exist, define it directly
125-
if (change.define === true && !(key in api)) {
126-
// Ensure descriptor has required boolean fields
127-
const defineDescriptor = {
128-
...descriptor,
129-
enumerable: typeof descriptor.enumerable !== 'boolean' ? true : descriptor.enumerable,
130-
configurable: typeof descriptor.configurable !== 'boolean' ? true : descriptor.configurable,
131-
};
132-
this.defineProperty(api, key, defineDescriptor);
133-
return;
153+
} else if (descriptorKind === 'getter') {
154+
// Mask the new setter against the original native setter (if any) so that
155+
// descriptor inspection (`getOwnPropertyDescriptor(...).set.toString()` /
156+
// `.set.name`) still resembles the original accessor. Event-handler IDL
157+
// attributes such as `Element.prototype.onclick` and
158+
// `MediaDevices.prototype.ondevicechange` expose a native setter; without
159+
// masking, descriptor probes would see our JS `setter(v) {...}` shape.
160+
const accessorDescriptor = /** @type {{ get?: () => any, set?: (v: any) => void }} */ (descriptor);
161+
if (typeof accessorDescriptor.set === 'function' && typeof origDescriptor.set === 'function') {
162+
accessorDescriptor.set = /** @type {(v: any) => void} */ (
163+
this.maskMethodReplacement(accessorDescriptor.set, origDescriptor.set)
164+
);
134165
}
166+
}
167+
if (hasOwnProperty.call(api, key)) {
135168
this.wrapProperty(api, key, descriptor);
169+
} else {
170+
// API exists via the prototype chain (e.g. MediaDevices.prototype.addEventListener
171+
// on EventTarget.prototype). Shadow-define an own property without requiring `define`.
172+
const merged = mergePropertyDescriptors(origDescriptor, descriptor);
173+
if (merged) {
174+
this.defineProperty(api, key, merged);
175+
}
176+
}
177+
}
178+
179+
/**
180+
* Returns the property descriptor for `key` on `obj` or an ancestor in its prototype chain.
181+
* @param {object} obj
182+
* @param {string} key
183+
* @returns {PropertyDescriptor | undefined}
184+
*/
185+
findPropertyDescriptor(obj, key) {
186+
let current = obj;
187+
while (current) {
188+
const descriptor = getOwnPropertyDescriptor(current, key);
189+
if (descriptor) {
190+
return descriptor;
191+
}
192+
current = Object.getPrototypeOf(current);
193+
}
194+
return undefined;
195+
}
196+
197+
/**
198+
* Wraps a config-supplied function so its observable identity (`toString`,
199+
* `toString.toString`, `name`, `length`) mirrors the original DOM method it is
200+
* replacing. The call itself still executes the configured replacement.
201+
*
202+
* Note: `processAttr` may return a shared function (e.g. `functionMap.noop`),
203+
* so we always create a fresh wrapper before redefining `name`/`length` to
204+
* avoid mutating module-level singletons.
205+
*
206+
* @param {Function} replacementFn - configured replacement to invoke
207+
* @param {Function} origFn - original DOM method we are masking against
208+
* @returns {Function}
209+
*/
210+
maskMethodReplacement(replacementFn, origFn) {
211+
// Use captured `ReflectApply` and `objectDefineProperty` so a hostile page
212+
// cannot intercept the call into the configured replacement, nor tamper with
213+
// the `name`/`length` masking, by reassigning `globalThis.Reflect.apply` or
214+
// `globalThis.Object.defineProperty` after content scope has initialised.
215+
const wrapper = function () {
216+
return ReflectApply(replacementFn, this, arguments);
217+
};
218+
objectDefineProperty(wrapper, 'name', { value: origFn.name, configurable: true });
219+
objectDefineProperty(wrapper, 'length', { value: origFn.length, configurable: true });
220+
return wrapToString(wrapper, origFn);
221+
}
222+
223+
/**
224+
* @param {'getter' | 'value'} descriptorKind
225+
* @param {import('../utils.js').ConfigSetting | import('../utils.js').ConfigSetting[] | undefined} configSetting
226+
* @param {APIChange} change
227+
* @returns {Partial<import('../wrapper-utils.js').StrictPropertyDescriptor>}
228+
*/
229+
createApiDescriptor(descriptorKind, configSetting, change) {
230+
// `Partial<StrictPropertyDescriptor>` is a union (data | accessor | get-only | set-only),
231+
// so direct property access narrows poorly. Build with a permissive intermediate shape
232+
// then return as the public type.
233+
/** @type {{ value?: any, get?: () => any, set?: (v: any) => void, enumerable?: boolean, configurable?: boolean }} */
234+
let descriptor;
235+
if (descriptorKind === 'value') {
236+
// `wrapApiDescriptor` guarantees `configSetting` is defined for the value kind.
237+
const valueSetting = /** @type {import('../utils.js').ConfigSetting | import('../utils.js').ConfigSetting[]} */ (configSetting);
238+
descriptor = { value: processAttr(valueSetting, undefined) };
239+
} else {
240+
descriptor = {};
241+
// `configSetting` is the getterValue (may be undefined if only setterValue is set).
242+
if (configSetting !== undefined) {
243+
const getterSetting = configSetting;
244+
descriptor.get = () => processAttr(getterSetting, undefined);
245+
}
246+
// `setterValue` is intentionally invoked through processAttr on every assignment so
247+
// configurations using `functionMap.debug` log per-assignment. When omitted, we leave
248+
// `set` unset on the new descriptor so wrapProperty's spread merge preserves the
249+
// original setter (existing behaviour).
250+
if (change.setterValue !== undefined) {
251+
const setterSetting = /** @type {import('../utils.js').ConfigSetting} */ (change.setterValue);
252+
descriptor.set = function setter(v) {
253+
const fn = processAttr(setterSetting, undefined);
254+
if (typeof fn === 'function') {
255+
ReflectApply(fn, this, [v]);
256+
}
257+
};
258+
}
259+
}
260+
if ('enumerable' in change) {
261+
descriptor.enumerable = change.enumerable;
262+
}
263+
if ('configurable' in change) {
264+
descriptor.configurable = change.configurable;
265+
}
266+
return /** @type {Partial<import('../wrapper-utils.js').StrictPropertyDescriptor>} */ (descriptor);
267+
}
268+
269+
/**
270+
* @param {Partial<import('../wrapper-utils.js').StrictPropertyDescriptor>} descriptor
271+
* @param {'getter' | 'value'} descriptorKind
272+
* @returns {import('../wrapper-utils.js').StrictPropertyDescriptor}
273+
*/
274+
createDefineDescriptor(descriptor, descriptorKind) {
275+
if (descriptorKind === 'value') {
276+
const valueDescriptor = /** @type {{ value: any, enumerable?: boolean, configurable?: boolean }} */ (descriptor);
277+
return {
278+
value: valueDescriptor.value,
279+
writable: true,
280+
enumerable: typeof valueDescriptor.enumerable !== 'boolean' ? true : valueDescriptor.enumerable,
281+
configurable: typeof valueDescriptor.configurable !== 'boolean' ? true : valueDescriptor.configurable,
282+
};
283+
}
284+
const getterDescriptor = /** @type {{ get?: () => any, set?: (v: any) => void, enumerable?: boolean, configurable?: boolean }} */ (
285+
descriptor
286+
);
287+
/** @type {any} */
288+
const result = {
289+
enumerable: typeof getterDescriptor.enumerable !== 'boolean' ? true : getterDescriptor.enumerable,
290+
configurable: typeof getterDescriptor.configurable !== 'boolean' ? true : getterDescriptor.configurable,
291+
};
292+
if (typeof getterDescriptor.get === 'function') {
293+
result.get = getterDescriptor.get;
294+
}
295+
if (typeof getterDescriptor.set === 'function') {
296+
result.set = getterDescriptor.set;
136297
}
298+
return result;
137299
}
138300

139301
/**

injected/src/wrapper-utils.js

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,45 @@ export function wrapFunction(functionValue, realTarget) {
111111
});
112112
}
113113

114+
/**
115+
* Merge `partial` into `orig` only when the partial supplies the same descriptor kind
116+
* (data `value` vs accessor `get`/`set`). Spreading incompatible shapes produces an
117+
* invalid ECMAScript property descriptor and makes `Object.defineProperty` throw.
118+
*
119+
* @param {PropertyDescriptor} origDescriptor
120+
* @param {Partial<PropertyDescriptor>} partialDescriptor
121+
* @returns {import('./wrapper-utils').StrictPropertyDescriptor | undefined} merged descriptor, or undefined when kinds disagree
122+
*/
123+
export function mergePropertyDescriptors(origDescriptor, partialDescriptor) {
124+
if (
125+
('value' in origDescriptor && 'value' in partialDescriptor) ||
126+
('get' in origDescriptor && 'get' in partialDescriptor) ||
127+
('set' in origDescriptor && 'set' in partialDescriptor)
128+
) {
129+
const merged = {
130+
...origDescriptor,
131+
...partialDescriptor,
132+
};
133+
// DOM descriptors always include configurable/enumerable at runtime; default when absent
134+
// so the result satisfies StrictPropertyDescriptor for defineProperty().
135+
if ('value' in merged) {
136+
return /** @type {import('./wrapper-utils').StrictPropertyDescriptor} */ ({
137+
value: merged.value,
138+
writable: typeof merged.writable === 'boolean' ? merged.writable : true,
139+
configurable: typeof merged.configurable === 'boolean' ? merged.configurable : true,
140+
enumerable: typeof merged.enumerable === 'boolean' ? merged.enumerable : true,
141+
});
142+
}
143+
return /** @type {import('./wrapper-utils').StrictPropertyDescriptor} */ ({
144+
get: merged.get,
145+
set: merged.set,
146+
configurable: typeof merged.configurable === 'boolean' ? merged.configurable : true,
147+
enumerable: typeof merged.enumerable === 'boolean' ? merged.enumerable : true,
148+
});
149+
}
150+
return undefined;
151+
}
152+
114153
/**
115154
* Wrap a `get`/`set` or `value` property descriptor. Only for data properties. For methods, use wrapMethod(). For constructors, use wrapConstructor().
116155
* @param {object} object - object whose property we are wrapping (most commonly a prototype, e.g. globalThis.Screen.prototype)
@@ -132,20 +171,13 @@ export function wrapProperty(object, propertyName, descriptor, definePropertyFn)
132171
return;
133172
}
134173

135-
if (
136-
('value' in origDescriptor && 'value' in descriptor) ||
137-
('get' in origDescriptor && 'get' in descriptor) ||
138-
('set' in origDescriptor && 'set' in descriptor)
139-
) {
140-
definePropertyFn(object, propertyName, {
141-
...origDescriptor,
142-
...descriptor,
143-
});
144-
return origDescriptor;
145-
} else {
174+
const merged = mergePropertyDescriptors(origDescriptor, descriptor);
175+
if (!merged) {
146176
// if the property is defined with get/set it must be wrapped with a get/set. If it's defined with a `value`, it must be wrapped with a `value`
147177
throw new Error(`Property descriptor for ${propertyName} may only include the following keys: ${objectKeys(origDescriptor)}`);
148178
}
179+
definePropertyFn(object, propertyName, merged);
180+
return origDescriptor;
149181
}
150182

151183
/**

0 commit comments

Comments
 (0)