module: do not check circular dependencies for exported proxies

In case the exported module is a proxy that has the `getPrototypeOf`
or `setPrototypeOf` trap, skip the circular dependencies check.
It would otherwise be triggered by the check itself.

Fixes: https://github.com/nodejs/node/issues/33334

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: https://github.com/nodejs/node/pull/33338
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
pull/33413/head
Ruben Bridgewater 2020-05-10 13:49:23 +02:00
parent 50ba066921
commit 24bf1adacc
4 changed files with 45 additions and 5 deletions

View File

@ -107,6 +107,11 @@ const {
CHAR_COLON
} = require('internal/constants');
const {
isProxy
} = require('internal/util/types');
const isWindows = process.platform === 'win32';
const relativeResolveCache = ObjectCreate(null);
@ -821,7 +826,7 @@ function emitCircularRequireWarning(prop) {
}
// A Proxy that can be used as the prototype of a module.exports object and
// warns when non-existend properties are accessed.
// warns when non-existent properties are accessed.
const CircularRequirePrototypeWarningProxy = new Proxy({}, {
get(target, prop) {
// Allow __esModule access in any case because it is used in the output
@ -840,20 +845,22 @@ const CircularRequirePrototypeWarningProxy = new Proxy({}, {
}
});
// Object.prototype and ObjectProtoype refer to our 'primordials' versions
// Object.prototype and ObjectPrototype refer to our 'primordials' versions
// and are not identical to the versions on the global object.
const PublicObjectPrototype = global.Object.prototype;
function getExportsForCircularRequire(module) {
if (module.exports &&
!isProxy(module.exports) &&
ObjectGetPrototypeOf(module.exports) === PublicObjectPrototype &&
// Exclude transpiled ES6 modules / TypeScript code because those may
// employ unusual patterns for accessing 'module.exports'. That should be
// okay because ES6 modules have a different approach to circular
// employ unusual patterns for accessing 'module.exports'. That should
// be okay because ES6 modules have a different approach to circular
// dependencies anyway.
!module.exports.__esModule) {
// This is later unset once the module is done loading.
ObjectSetPrototypeOf(module.exports, CircularRequirePrototypeWarningProxy);
ObjectSetPrototypeOf(
module.exports, CircularRequirePrototypeWarningProxy);
}
return module.exports;
@ -943,6 +950,7 @@ Module._load = function(request, parent, isMain) {
}
}
} else if (module.exports &&
!isProxy(module.exports) &&
ObjectGetPrototypeOf(module.exports) ===
CircularRequirePrototypeWarningProxy) {
ObjectSetPrototypeOf(module.exports, PublicObjectPrototype);

View File

@ -0,0 +1,17 @@
module.exports = new Proxy({}, {
get(_target, prop) { throw new Error(`get: ${String(prop)}`); },
getPrototypeOf() { throw new Error('getPrototypeOf'); },
setPrototypeOf() { throw new Error('setPrototypeOf'); },
isExtensible() { throw new Error('isExtensible'); },
preventExtensions() { throw new Error('preventExtensions'); },
getOwnPropertyDescriptor() { throw new Error('getOwnPropertyDescriptor'); },
defineProperty() { throw new Error('defineProperty'); },
has() { throw new Error('has'); },
set() { throw new Error('set'); },
deleteProperty() { throw new Error('deleteProperty'); },
ownKeys() { throw new Error('ownKeys'); },
apply() { throw new Error('apply'); },
construct() { throw new Error('construct'); }
});
require('./warning-skip-proxy-traps-b.js');

View File

@ -0,0 +1,10 @@
const assert = require('assert');
const object = require('./warning-skip-proxy-traps-a.js');
assert.throws(() => {
object.missingPropProxyTrap;
}, {
message: 'get: missingPropProxyTrap',
name: 'Error',
});

View File

@ -38,3 +38,8 @@ assert.strictEqual(esmTranspiledExport.__esModule, true);
const halfTranspiledExport =
require(fixtures.path('cycles', 'warning-esm-half-transpiled-a.js'));
assert.strictEqual(halfTranspiledExport.__esModule, undefined);
// No circular check is done to prevent triggering proxy traps, if
// module.exports is set to a proxy that contains a `getPrototypeOf` or
// `setPrototypeOf` trap.
require(fixtures.path('cycles', 'warning-skip-proxy-traps-a.js'));