From 5518664d41b8916dfbe2eca2180d760db632748e Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 9 May 2019 09:22:15 +0200
Subject: [PATCH] util: if present, fallback to `toString` using the %s
 formatter

This makes sure that `util.format` uses `String` to stringify an object
in case the object has an own property named `toString` with type
`function`. That way objects that do not have such function are still
inspected using `util.inspect` and the old behavior is preserved as
well.

PR-URL: https://github.com/nodejs/node/pull/27621
Refs: https://github.com/facebook/jest/issues/8443
Reviewed-By: Roman Reiss <me@silverwind.io>
---
 doc/api/util.md                   |  6 ++---
 lib/internal/util/inspect.js      | 39 ++++++++++++++++++++++++-------
 test/parallel/test-util-format.js | 22 +++++++++++++++++
 3 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/doc/api/util.md b/doc/api/util.md
index e0b30e9a97fdcd..301089fd47c990 100644
--- a/doc/api/util.md
+++ b/doc/api/util.md
@@ -221,9 +221,9 @@ specifiers. Each specifier is replaced with the converted value from the
 corresponding argument. Supported specifiers are:
 
 * `%s` - `String` will be used to convert all values except `BigInt`, `Object`
-  and `-0`. `BigInt` values will be represented with an `n` and Objects are
-  inspected using `util.inspect()` with options
- `{ depth: 0, colors: false, compact: 3 }`.
+  and `-0`. `BigInt` values will be represented with an `n` and Objects that
+  have no user defined `toString` function are inspected using `util.inspect()`
+  with options `{ depth: 0, colors: false, compact: 3 }`.
 * `%d` - `Number` will be used to convert all values except `BigInt` and
   `Symbol`.
 * `%i` - `parseInt(value, 10)` is used for all values except `BigInt` and
diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index 07757f3fe0b6ad..e46a18633cbc87 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -93,6 +93,10 @@ const { NativeModule } = require('internal/bootstrap/loaders');
 
 let hexSlice;
 
+const builtInObjects = new Set(
+  Object.getOwnPropertyNames(global).filter((e) => /^([A-Z][a-z]+)+$/.test(e))
+);
+
 const inspectDefaultOptions = Object.seal({
   showHidden: false,
   depth: 2,
@@ -1541,16 +1545,33 @@ function formatWithOptions(inspectOptions, ...args) {
           switch (nextChar) {
             case 115: // 's'
               const tempArg = args[++a];
-              if (typeof tempArg !== 'string' &&
-                  typeof tempArg !== 'function') {
-                tempStr = inspect(tempArg, {
-                  ...inspectOptions,
-                  compact: 3,
-                  colors: false,
-                  depth: 0
-                });
+              if (typeof tempArg === 'number') {
+                tempStr = formatNumber(stylizeNoColor, tempArg);
+              // eslint-disable-next-line valid-typeof
+              } else if (typeof tempArg === 'bigint') {
+                tempStr = `${tempArg}n`;
               } else {
-                tempStr = String(tempArg);
+                let constr;
+                if (typeof tempArg !== 'object' ||
+                    tempArg === null ||
+                    typeof tempArg.toString === 'function' &&
+                     // A direct own property.
+                     (hasOwnProperty(tempArg, 'toString') ||
+                      // A direct own property on the constructor prototype in
+                      // case the constructor is not an built-in object.
+                      (constr = tempArg.constructor) &&
+                      !builtInObjects.has(constr.name) &&
+                      constr.prototype &&
+                      hasOwnProperty(constr.prototype, 'toString'))) {
+                  tempStr = String(tempArg);
+                } else {
+                  tempStr = inspect(tempArg, {
+                    ...inspectOptions,
+                    compact: 3,
+                    colors: false,
+                    depth: 0
+                  });
+                }
               }
               break;
             case 106: // 'j'
diff --git a/test/parallel/test-util-format.js b/test/parallel/test-util-format.js
index fa0f66ecfc3fa1..5b012dd64d58d5 100644
--- a/test/parallel/test-util-format.js
+++ b/test/parallel/test-util-format.js
@@ -138,8 +138,30 @@ assert.strictEqual(util.format('%s', 42n), '42n');
 assert.strictEqual(util.format('%s', Symbol('foo')), 'Symbol(foo)');
 assert.strictEqual(util.format('%s', true), 'true');
 assert.strictEqual(util.format('%s', { a: [1, 2, 3] }), '{ a: [Array] }');
+assert.strictEqual(util.format('%s', { toString() { return 'Foo'; } }), 'Foo');
+assert.strictEqual(util.format('%s', { toString: 5 }), '{ toString: 5 }');
 assert.strictEqual(util.format('%s', () => 5), '() => 5');
 
+// String format specifier including `toString` properties on the prototype.
+{
+  class Foo { toString() { return 'Bar'; } }
+  assert.strictEqual(util.format('%s', new Foo()), 'Bar');
+  assert.strictEqual(
+    util.format('%s', Object.setPrototypeOf(new Foo(), null)),
+    '[Foo: null prototype] {}'
+  );
+  global.Foo = Foo;
+  assert.strictEqual(util.format('%s', new Foo()), 'Bar');
+  delete global.Foo;
+  class Bar { abc = true; }
+  assert.strictEqual(util.format('%s', new Bar()), 'Bar { abc: true }');
+  class Foobar extends Array { aaa = true; }
+  assert.strictEqual(
+    util.format('%s', new Foobar(5)),
+    'Foobar [ <5 empty items>, aaa: true ]'
+  );
+}
+
 // JSON format specifier
 assert.strictEqual(util.format('%j'), '%j');
 assert.strictEqual(util.format('%j', 42), '42');