diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 438bde842d8b..40c9afac9a5d 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1035,11 +1035,16 @@ function determineSpecificType(value) { return `type symbol (${String(value)})`; case 'function': return `function ${value.name}`; - case 'object': - if (value.constructor && 'name' in value.constructor) { - return `an instance of ${value.constructor.name}`; + case 'object': { + // `constructor` may be user-controlled: it need not be an object, and + // its `name` need not be a non-empty string. Reading either can invoke + // an accessor, so read each one once. + const name = value.constructor?.name; + if (typeof name === 'string' && name !== '') { + return `an instance of ${name}`; } return `${lazyInternalUtilInspect().inspect(value, { depth: -1 })}`; + } case 'string': value.length > 28 && (value = `${StringPrototypeSlice(value, 0, 25)}...`); if (StringPrototypeIndexOf(value, "'") === -1) { diff --git a/test/common/index.js b/test/common/index.js index e37b354f8259..3ec677e566d8 100755 --- a/test/common/index.js +++ b/test/common/index.js @@ -855,8 +855,9 @@ function invalidArgTypeHelper(input) { return ` Received function ${input.name}`; } if (typeof input === 'object') { - if (input.constructor?.name) { - return ` Received an instance of ${input.constructor.name}`; + const name = input.constructor?.name; + if (typeof name === 'string' && name !== '') { + return ` Received an instance of ${name}`; } return ` Received ${inspect(input, { depth: -1 })}`; } diff --git a/test/parallel/test-error-value-type-detection.mjs b/test/parallel/test-error-value-type-detection.mjs index e44b19297a96..cb794514ed39 100644 --- a/test/parallel/test-error-value-type-detection.mjs +++ b/test/parallel/test-error-value-type-detection.mjs @@ -209,3 +209,58 @@ assert.strictEqual( determineSpecificType(new WeakSet()), 'an instance of WeakSet', ); + +// Anonymous classes have an empty `name`, so the value is inspected instead. +// `inspect(..., { depth: -1 })` prints this one as `{}` because it has no own +// properties; the values below carry a `constructor` property of their own and +// print as `[Object]`. +assert.strictEqual( + determineSpecificType(new (class {})()), + '{}', +); + +// `constructor` is an ordinary, user-controlled property that need not be a +// function. Describing such a value must not throw. +assert.strictEqual( + determineSpecificType(JSON.parse('{"constructor": 5}')), + '[Object]', +); + +assert.strictEqual( + determineSpecificType({ constructor: { name: '' } }), + '[Object]', +); + +// A `constructor.name` that is not a usable string must not be interpolated +// into the message. +assert.strictEqual( + determineSpecificType({ constructor: { name: Symbol('x') } }), + '[Object]', +); + +assert.strictEqual( + determineSpecificType({ constructor: { name: 42 } }), + '[Object]', +); + +// `constructor` and its `name` are each read once. Both may be accessors, so +// repeated reads are observable, and a `name` validated by one read and +// interpolated from another need not be the same value twice. +let constructorReads = 0; +let nameReads = 0; +const named = { + get constructor() { + constructorReads++; + return { get name() { nameReads++; return 'Foo'; } }; + }, +}; +assert.strictEqual(determineSpecificType(named), 'an instance of Foo'); +assert.strictEqual(constructorReads, 1); +assert.strictEqual(nameReads, 1); + +// Building the error must not fail when `constructor` is a truthy primitive. +assert.strictEqual( + new errorsModule.codes.ERR_INVALID_ARG_TYPE( + 'arg', 'string', JSON.parse('{"constructor": 5}')).code, + 'ERR_INVALID_ARG_TYPE', +);