util: fix inspection of class instance prototypes

To achieve this, some internal custom inspect functions had to be changed. They relied upon the former behavior. Signed-off-by: Ruben Bridgewater <[email protected]> PR-URL: #33449 Fixes: #33419 Reviewed-By: Anto Aravinth <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
nodejs · Jun 30, 2020 · f0bca4a · f0bca4a
1 parent 35d6800
commit f0bca4a
Show file tree

Hide file tree

Showing 8 changed files with 113 additions and 72 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -266,7 +266,7 @@ assert.strictEqual(
   '  func: <ref *1> [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: <ref *1> [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: <ref *1> [Function: func] {\n' +
   '      [length]: 0,\n' +
   '      [name]: \'func\',\n' +
-  '      [prototype]: func { [constructor]: [Circular *1] }\n' +
+  '      [prototype]: { [constructor]: [Circular *1] }\n' +
   '    }\n' +
   '  }\n' +
   '}');
@@ -306,15 +306,15 @@ assert.strictEqual(
   '  func: <ref *1> [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' +
   '  foobar: 1,\n' +
   '  func: <ref *1> [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: <ref *1> [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
@@ -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 }),
+    '<ref *1> Foo [Map] {\n' +
+    '    [constructor]: [class Bar extends Foo] {\n' +
+    '      [length]: 0,\n' +
+    '      [prototype]: [Circular *1],\n' +
+    "      [name]: 'Bar',\n" +
+    '      [Symbol(Symbol.species)]: [Getter: <Inspection threw ' +
+      "(Symbol.prototype.toString requires that 'this' be a Symbol)>]\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
@@ -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
@@ -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
@@ -125,10 +126,7 @@ if (common.hasIntl) {
       '  [Symbol(flags)]: 4,\n' +
       '  [Symbol(handle)]: StringDecoder {\n' +
       "    encoding: 'utf8',\n" +
-      '    [Symbol(kNativeDecoder)]: <Buffer 00 00 00 00 00 00 01>,\n' +
-      '    lastChar: [Getter],\n' +
-      '    lastNeed: [Getter],\n' +
-      '    lastTotal: [Getter]\n' +
+      '    [Symbol(kNativeDecoder)]: <Buffer 00 00 00 00 00 00 01>\n' +
       '  }\n' +
       '}'
     );