Skip to content

Commit 19699be

Browse files
committed
test_runner: improve mock.module options handling
This commit addresses several feedback items for mock.module(): - Fixes a potential prototype pollution vulnerability by using ObjectAssign({ __proto__: null }, ...) for property descriptors. - Migrates legacy option warnings to use the internal deprecateProperty() utility for better consistency. - Updates documentation to use "a later version" instead of "a future major release" for deprecation timelines, as the feature is experimental. - Adds comprehensive test cases to ensure getter properties in mock options are correctly preserved across both module systems.
1 parent 7c2fd5c commit 19699be

File tree

3 files changed

+57
-23
lines changed

3 files changed

+57
-23
lines changed

doc/api/test.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2497,14 +2497,14 @@ changes:
24972497
export. If the mock is a CommonJS or builtin module, this setting is used as
24982498
the value of `module.exports`. If this value is not provided, CJS and builtin
24992499
mocks use an empty object as the value of `module.exports`.
2500-
This option is deprecated and will be removed in a future major release.
2500+
This option is deprecated and will be removed in a later version.
25012501
Prefer `options.exports.default`.
25022502
* `namedExports` {Object} An optional object whose keys and values are used to
25032503
create the named exports of the mock module. If the mock is a CommonJS or
25042504
builtin module, these values are copied onto `module.exports`. Therefore, if a
25052505
mock is created with both named exports and a non-object default export, the
25062506
mock will throw an exception when used as a CJS or builtin module.
2507-
This option is deprecated and will be removed in a future major release.
2507+
This option is deprecated and will be removed in a later version.
25082508
Prefer `options.exports`.
25092509
* Returns: {MockModuleContext} An object that can be used to manipulate the mock.
25102510

lib/internal/test_runner/mock/mock.js

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const {
66
Error,
77
FunctionPrototypeBind,
88
FunctionPrototypeCall,
9+
ObjectAssign,
910
ObjectDefineProperty,
1011
ObjectGetOwnPropertyDescriptor,
1112
ObjectGetPrototypeOf,
@@ -34,6 +35,7 @@ const {
3435
URLParse,
3536
} = require('internal/url');
3637
const {
38+
deprecateProperty,
3739
emitExperimentalWarning,
3840
getStructuredStack,
3941
kEmptyObject,
@@ -62,12 +64,14 @@ const kSupportedFormats = [
6264
'module',
6365
];
6466
let sharedModuleState;
65-
const warnedLegacyOptions = { __proto__: null };
66-
const kLegacyOptionReplacements = {
67-
__proto__: null,
68-
defaultExport: 'options.exports.default',
69-
namedExports: 'options.exports',
70-
};
67+
const deprecateNamedExports = deprecateProperty(
68+
'namedExports',
69+
'mock.module(): options.namedExports is deprecated. Use options.exports instead.',
70+
);
71+
const deprecateDefaultExport = deprecateProperty(
72+
'defaultExport',
73+
'mock.module(): options.defaultExport is deprecated. Use options.exports.default instead.',
74+
);
7175
const {
7276
hooks: mockHooks,
7377
mocks,
@@ -815,6 +819,9 @@ function normalizeModuleMockOptions(options) {
815819
const { cache = false } = options;
816820
validateBoolean(cache, 'options.cache');
817821

822+
deprecateNamedExports(options);
823+
deprecateDefaultExport(options);
824+
818825
const moduleExports = { __proto__: null };
819826

820827
if ('exports' in options) {
@@ -824,16 +831,15 @@ function normalizeModuleMockOptions(options) {
824831

825832
if ('namedExports' in options) {
826833
validateObject(options.namedExports, 'options.namedExports');
827-
emitLegacyMockOptionWarning('namedExports');
828834
copyOwnProperties(options.namedExports, moduleExports);
829835
}
830836

831837
if ('defaultExport' in options) {
832-
emitLegacyMockOptionWarning('defaultExport');
833-
ObjectDefineProperty(moduleExports, 'default', {
834-
__proto__: null,
835-
...ObjectGetOwnPropertyDescriptor(options, 'defaultExport'),
836-
});
838+
ObjectDefineProperty(
839+
moduleExports,
840+
'default',
841+
ObjectAssign({ __proto__: null }, ObjectGetOwnPropertyDescriptor(options, 'defaultExport')),
842+
);
837843
}
838844

839845
return {
@@ -843,15 +849,6 @@ function normalizeModuleMockOptions(options) {
843849
};
844850
}
845851

846-
function emitLegacyMockOptionWarning(option) {
847-
if (warnedLegacyOptions[option]) return;
848-
warnedLegacyOptions[option] = true;
849-
process.emitWarning(
850-
`mock.module(): options.${option} is deprecated. ` +
851-
`Use ${kLegacyOptionReplacements[option]} instead.`,
852-
'DeprecationWarning',
853-
);
854-
}
855852

856853
function copyOwnProperties(from, to) {
857854
const keys = ObjectKeys(from);

test/parallel/test-runner-module-mocking.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,43 @@ test('exports option works with core module mocks in both module systems', async
690690
});
691691
});
692692

693+
async function assertGetterMockWorksInBothSystems(t, mockOptionsFactory) {
694+
const fixturePath = fixtures.path('module-mocking', 'basic-esm.mjs');
695+
const fixture = pathToFileURL(fixturePath);
696+
const original = await import(fixture);
697+
let getterCalls = 0;
698+
699+
assert.strictEqual(original.string, 'original esm string');
700+
701+
const options = mockOptionsFactory(() => {
702+
getterCalls++;
703+
return { mocked: true };
704+
});
705+
706+
t.mock.module(`${fixture}`, options);
707+
708+
assert.deepStrictEqual((await import(fixture)).default, { mocked: true });
709+
assert.deepStrictEqual(require(fixturePath), { mocked: true });
710+
assert.strictEqual(getterCalls, 2);
711+
}
712+
713+
test('defaultExports getter works in both module systems', async (t) => {
714+
await assertGetterMockWorksInBothSystems(t, (getter) => ({
715+
get defaultExport() {
716+
return getter();
717+
},
718+
}));
719+
});
720+
721+
test('exports.default getter works in both module systems', async (t) => {
722+
await assertGetterMockWorksInBothSystems(t, (getter) => ({
723+
exports: {
724+
get default() {
725+
return getter();
726+
},
727+
},
728+
}));
729+
});
693730
test('exports option supports default for CJS mocks in both module systems', async (t) => {
694731
const fixturePath = fixtures.path('module-mocking', 'basic-cjs.js');
695732
const fixture = pathToFileURL(fixturePath);

0 commit comments

Comments
 (0)