From 8bb164dc00bf11f9cb0ef58b761a65fcb7325a13 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 17 May 2020 19:07:04 +0200 Subject: [PATCH 1/2] util: fix inspection of class instance prototypes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To achieve this, some internal custom inspect functions had to be changed. They relied upon the former behavior. Fixes: https://github.com/nodejs/node/issues/33419 Signed-off-by: Ruben Bridgewater PR-URL: https://github.com/nodejs/node/pull/33449 Reviewed-By: Anto Aravinth Reviewed-By: Michaël Zasso --- lib/internal/encoding.js | 91 +++++++++---------- lib/internal/url.js | 9 +- lib/internal/util/inspect.js | 3 +- lib/internal/vm/module.js | 24 +++-- test/parallel/test-util-format.js | 12 +-- test/parallel/test-util-inspect.js | 31 +++++++ test/parallel/test-vm-module-basic.js | 9 +- ...test-whatwg-encoding-custom-textdecoder.js | 1 + 8 files changed, 112 insertions(+), 68 deletions(-) diff --git a/lib/internal/encoding.js b/lib/internal/encoding.js index befc4f811b2dde..de0a1b7cc799bb 100644 --- a/lib/internal/encoding.js +++ b/lib/internal/encoding.js @@ -514,54 +514,53 @@ function makeTextDecoderJS() { } // Mix in some shared properties. -{ - ObjectDefineProperties( - TextDecoder.prototype, - ObjectGetOwnPropertyDescriptors({ - get encoding() { - validateDecoder(this); - return this[kEncoding]; - }, - - get fatal() { - validateDecoder(this); - return (this[kFlags] & CONVERTER_FLAGS_FATAL) === CONVERTER_FLAGS_FATAL; - }, - - get ignoreBOM() { - validateDecoder(this); - return (this[kFlags] & CONVERTER_FLAGS_IGNORE_BOM) === - CONVERTER_FLAGS_IGNORE_BOM; - }, - - [inspect](depth, opts) { - validateDecoder(this); - if (typeof depth === 'number' && depth < 0) - return this; - const ctor = getConstructorOf(this); - const obj = ObjectCreate({ - constructor: ctor === null ? TextDecoder : ctor - }); - obj.encoding = this.encoding; - obj.fatal = this.fatal; - obj.ignoreBOM = this.ignoreBOM; - if (opts.showHidden) { - obj[kFlags] = this[kFlags]; - obj[kHandle] = this[kHandle]; - } - // Lazy to avoid circular dependency - return require('internal/util/inspect').inspect(obj, opts); +ObjectDefineProperties( + TextDecoder.prototype, + ObjectGetOwnPropertyDescriptors({ + get encoding() { + validateDecoder(this); + return this[kEncoding]; + }, + + get fatal() { + validateDecoder(this); + return (this[kFlags] & CONVERTER_FLAGS_FATAL) === CONVERTER_FLAGS_FATAL; + }, + + get ignoreBOM() { + validateDecoder(this); + return (this[kFlags] & CONVERTER_FLAGS_IGNORE_BOM) === + CONVERTER_FLAGS_IGNORE_BOM; + }, + + [inspect](depth, opts) { + validateDecoder(this); + if (typeof depth === 'number' && depth < 0) + return this; + const constructor = getConstructorOf(this) || TextDecoder; + const obj = ObjectCreate({ constructor }); + obj.encoding = this.encoding; + obj.fatal = this.fatal; + obj.ignoreBOM = this.ignoreBOM; + if (opts.showHidden) { + obj[kFlags] = this[kFlags]; + obj[kHandle] = this[kHandle]; } - })); - ObjectDefineProperties(TextDecoder.prototype, { - decode: { enumerable: true }, - [inspect]: { enumerable: false }, - [SymbolToStringTag]: { - configurable: true, - value: 'TextDecoder' + // Lazy to avoid circular dependency + const { inspect } = require('internal/util/inspect'); + return `${constructor.name} ${inspect(obj)}`; } - }); -} + }) +); + +ObjectDefineProperties(TextDecoder.prototype, { + decode: { enumerable: true }, + [inspect]: { enumerable: false }, + [SymbolToStringTag]: { + configurable: true, + value: 'TextDecoder' + } +}); module.exports = { getEncodingFromLabel, diff --git a/lib/internal/url.js b/lib/internal/url.js index 78f5b32745a043..c0b8c17d098708 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -358,11 +358,8 @@ class URL { if (typeof depth === 'number' && depth < 0) return this; - const ctor = getConstructorOf(this); - - const obj = ObjectCreate({ - constructor: ctor === null ? URL : ctor - }); + const constructor = getConstructorOf(this) || URL; + const obj = ObjectCreate({ constructor }); obj.href = this.href; obj.origin = this.origin; @@ -383,7 +380,7 @@ class URL { obj[context] = this[context]; } - return inspect(obj, opts); + return `${constructor.name} ${inspect(obj, opts)}`; } } diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 44d588fe3b7aa6..fbc0a4ee2d6b6e 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -498,7 +498,8 @@ function getConstructorName(obj, ctx, recurseTimes, protoProps) { const descriptor = ObjectGetOwnPropertyDescriptor(obj, 'constructor'); if (descriptor !== undefined && typeof descriptor.value === 'function' && - descriptor.value.name !== '') { + descriptor.value.name !== '' && + tmp instanceof descriptor.value) { if (protoProps !== undefined && (firstProto !== obj || !builtInObjects.has(descriptor.value.name))) { diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index 1d26e7a601b5bc..d9892d520e78fe 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -5,6 +5,8 @@ const { ArrayIsArray, ObjectCreate, ObjectDefineProperty, + ObjectGetPrototypeOf, + ObjectSetPrototypeOf, SafePromise, Symbol, WeakMap, @@ -223,17 +225,27 @@ class Module { } [customInspectSymbol](depth, options) { - let ctor = getConstructorOf(this); - ctor = ctor === null ? Module : ctor; - + if (this[kWrap] === undefined) { + throw new ERR_VM_MODULE_NOT_MODULE(); + } if (typeof depth === 'number' && depth < 0) - return options.stylize(`[${ctor.name}]`, 'special'); + return this; - const o = ObjectCreate({ constructor: ctor }); + const constructor = getConstructorOf(this) || Module; + const o = ObjectCreate({ constructor }); o.status = this.status; o.identifier = this.identifier; o.context = this.context; - return require('internal/util/inspect').inspect(o, options); + + ObjectSetPrototypeOf(o, ObjectGetPrototypeOf(this)); + ObjectDefineProperty(o, Symbol.toStringTag, { + value: constructor.name, + configurable: true + }); + + // Lazy to avoid circular dependency + const { inspect } = require('internal/util/inspect'); + return inspect(o, { ...options, customInspect: false }); } } diff --git a/test/parallel/test-util-format.js b/test/parallel/test-util-format.js index 6de9d8d7c74c80..11fe24fce70770 100644 --- a/test/parallel/test-util-format.js +++ b/test/parallel/test-util-format.js @@ -266,7 +266,7 @@ assert.strictEqual( ' func: [Function: func] {\n' + ' [length]: 0,\n' + ' [name]: \'func\',\n' + - ' [prototype]: func { [constructor]: [Circular *1] }\n' + + ' [prototype]: { [constructor]: [Circular *1] }\n' + ' }\n' + '}'); assert.strictEqual( @@ -279,7 +279,7 @@ assert.strictEqual( ' a: [Function: a] {\n' + ' [length]: 0,\n' + ' [name]: \'a\',\n' + - ' [prototype]: a { [constructor]: [Circular *1] }\n' + + ' [prototype]: { [constructor]: [Circular *1] }\n' + ' }\n' + ' },\n' + ' [length]: 1\n' + @@ -294,7 +294,7 @@ assert.strictEqual( ' func: [Function: func] {\n' + ' [length]: 0,\n' + ' [name]: \'func\',\n' + - ' [prototype]: func { [constructor]: [Circular *1] }\n' + + ' [prototype]: { [constructor]: [Circular *1] }\n' + ' }\n' + ' }\n' + '}'); @@ -306,7 +306,7 @@ assert.strictEqual( ' func: [Function: func] {\n' + ' [length]: 0,\n' + ' [name]: \'func\',\n' + - ' [prototype]: func { [constructor]: [Circular *1] }\n' + + ' [prototype]: { [constructor]: [Circular *1] }\n' + ' }\n' + '} {\n' + ' foo: \'bar\',\n' + @@ -314,7 +314,7 @@ assert.strictEqual( ' func: [Function: func] {\n' + ' [length]: 0,\n' + ' [name]: \'func\',\n' + - ' [prototype]: func { [constructor]: [Circular *1] }\n' + + ' [prototype]: { [constructor]: [Circular *1] }\n' + ' }\n' + '}'); assert.strictEqual( @@ -325,7 +325,7 @@ assert.strictEqual( ' func: [Function: func] {\n' + ' [length]: 0,\n' + ' [name]: \'func\',\n' + - ' [prototype]: func { [constructor]: [Circular *1] }\n' + + ' [prototype]: { [constructor]: [Circular *1] }\n' + ' }\n' + '} %o'); diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 30acf9dbb050d7..1700e4d5c636aa 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -2827,6 +2827,37 @@ assert.strictEqual( '{ \x1B[2mabc: \x1B[33mtrue\x1B[39m\x1B[22m, ' + '\x1B[2mdef: \x1B[33m5\x1B[39m\x1B[22m }' ); + + assert.strictEqual( + inspect(Object.getPrototypeOf(bar), { showHidden: true, getters: true }), + ' Foo [Map] {\n' + + ' [constructor]: [class Bar extends Foo] {\n' + + ' [length]: 0,\n' + + ' [prototype]: [Circular *1],\n' + + " [name]: 'Bar',\n" + + ' [Symbol(Symbol.species)]: [Getter: ]\n" + + ' },\n' + + " [xyz]: [Getter: 'YES!'],\n" + + ' [Symbol(nodejs.util.inspect.custom)]: ' + + '[Function: [nodejs.util.inspect.custom]] {\n' + + ' [length]: 0,\n' + + " [name]: '[nodejs.util.inspect.custom]'\n" + + ' },\n' + + ' [abc]: [Getter: true],\n' + + ' [def]: [Getter/Setter: false]\n' + + ' }' + ); + + assert.strictEqual( + inspect(Object.getPrototypeOf(bar)), + 'Foo [Map] {}' + ); + + assert.strictEqual( + inspect(Object.getPrototypeOf(new Foo())), + 'Map {}' + ); } // Test changing util.inspect.colors colors and aliases. diff --git a/test/parallel/test-vm-module-basic.js b/test/parallel/test-vm-module-basic.js index 3e5e7759c1a9cb..7a669c8e05d741 100644 --- a/test/parallel/test-vm-module-basic.js +++ b/test/parallel/test-vm-module-basic.js @@ -83,9 +83,12 @@ const util = require('util'); assert.strictEqual(util.inspect(m, { depth: -1 }), '[SourceTextModule]'); - assert.strictEqual( - m[util.inspect.custom].call(Object.create(null)), - 'Module { status: undefined, identifier: undefined, context: undefined }', + assert.throws( + () => m[util.inspect.custom].call(Object.create(null)), + { + code: 'ERR_VM_MODULE_NOT_MODULE', + message: 'Provided module is not an instance of Module' + }, ); } diff --git a/test/parallel/test-whatwg-encoding-custom-textdecoder.js b/test/parallel/test-whatwg-encoding-custom-textdecoder.js index d16fd24bcd3739..9257ab537902dd 100644 --- a/test/parallel/test-whatwg-encoding-custom-textdecoder.js +++ b/test/parallel/test-whatwg-encoding-custom-textdecoder.js @@ -85,6 +85,7 @@ if (common.hasIntl) { assert.strictEqual(dec.encoding, 'utf-8'); assert.strictEqual(dec.fatal, false); assert.strictEqual(dec.ignoreBOM, false); + assert.strictEqual(dec[Symbol.toStringTag], 'TextDecoder'); } // Test TextDecoder, UTF-16le From 77e6691d442aa5d2b1a454b1cec4e7d61b8a336f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 23 May 2020 22:22:49 +0200 Subject: [PATCH 2/2] fixup! util: fix inspection of class instance prototypes Signed-off-by: Ruben Bridgewater --- test/parallel/test-whatwg-encoding-custom-textdecoder.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/parallel/test-whatwg-encoding-custom-textdecoder.js b/test/parallel/test-whatwg-encoding-custom-textdecoder.js index 9257ab537902dd..877cd43734451b 100644 --- a/test/parallel/test-whatwg-encoding-custom-textdecoder.js +++ b/test/parallel/test-whatwg-encoding-custom-textdecoder.js @@ -126,10 +126,7 @@ if (common.hasIntl) { ' [Symbol(flags)]: 4,\n' + ' [Symbol(handle)]: StringDecoder {\n' + " encoding: 'utf8',\n" + - ' [Symbol(kNativeDecoder)]: ,\n' + - ' lastChar: [Getter],\n' + - ' lastNeed: [Getter],\n' + - ' lastTotal: [Getter]\n' + + ' [Symbol(kNativeDecoder)]: \n' + ' }\n' + '}' );