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.
nodejs · May 16, 2019 · 0bb8a7f · 0bb8a7f
1 parent 6402452
commit 0bb8a7f
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 12 deletions.
diff --git 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
@@ -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,
@@ -1543,16 +1547,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
@@ -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');