util: fix infinite recursion during inspection

A specially crafted object with circular structures behind getters could cause a infinite recursion. This is now fixed by detecting the objects as already visited. Signed-off-by: Ruben Bridgewater <[email protected]> PR-URL: #37079 Fixes: #37054 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Anto Aravinth <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
nodejs · May 3, 2021 · ec5b06e · ec5b06e
1 parent 67e9e71
commit ec5b06e
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 15 deletions.
diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
@@ -5,6 +5,7 @@ const {
   ArrayIsArray,
   ArrayPrototypeFilter,
   ArrayPrototypeForEach,
+  ArrayPrototypePop,
   ArrayPrototypePush,
   ArrayPrototypePushApply,
   ArrayPrototypeSort,
@@ -620,6 +621,7 @@ function addPrototypeProperties(ctx, main, obj, recurseTimes, output) {
     }
     // Get all own property names and symbols.
     keys = ReflectOwnKeys(obj);
+    ArrayPrototypePush(ctx.seen, main);
     for (const key of keys) {
       // Ignore the `constructor` property and keys that exist on layers above.
       if (key === 'constructor' ||
@@ -640,6 +642,7 @@ function addPrototypeProperties(ctx, main, obj, recurseTimes, output) {
         ArrayPrototypePush(output, value);
       }
     }
+    ArrayPrototypePop(ctx.seen);
   // Limit the inspection to up to three prototype layers. Using `recurseTimes`
   // is not a good choice here, because it's as if the properties are declared
   // on the current object from the users perspective.

diff --git a/test/parallel/test-util-inspect-getters-accessing-this.js b/test/parallel/test-util-inspect-getters-accessing-this.js
@@ -7,24 +7,61 @@ require('../common');
 
 const assert = require('assert');
 
-const util = require('util');
+const { inspect } = require('util');
 
-class X {
-  constructor() {
-    this._y = 123;
-  }
+{
+  class X {
+    constructor() {
+      this._y = 123;
+    }
 
-  get y() {
-    return this._y;
+    get y() {
+      return this._y;
+    }
   }
+
+  const result = inspect(new X(), {
+    getters: true,
+    showHidden: true
+  });
+
+  assert.strictEqual(
+    result,
+    'X { _y: 123, [y]: [Getter: 123] }'
+  );
 }
 
-const result = util.inspect(new X(), {
-  getters: true,
-  showHidden: true
-});
+// Regression test for https://github.com/nodejs/node/issues/37054
+{
+  class A {
+    constructor(B) {
+      this.B = B;
+    }
+    get b() {
+      return this.B;
+    }
+  }
+
+  class B {
+    constructor() {
+      this.A = new A(this);
+    }
+    get a() {
+      return this.A;
+    }
+  }
+
+  const result = inspect(new B(), {
+    depth: 1,
+    getters: true,
+    showHidden: true
+  });
 
-assert.strictEqual(
-  result,
-  'X { _y: 123, [y]: [Getter: 123] }'
-);
+  assert.strictEqual(
+    result,
+    '<ref *1> B {\n' +
+    '  A: A { B: [Circular *1], [b]: [Getter] [Circular *1] },\n' +
+    '  [a]: [Getter] A { B: [Circular *1], [b]: [Getter] [Circular *1] }\n' +
+    '}',
+  );
+}