Commit a1ce776
util: fix .format() not always calling toString when it should be
This makes sure that `util.format('%s', object)` will always call
a user defined `toString` function. It was formerly not the case
when the object had the function declared on the super class.
At the same time this also makes sure that getters won't be
triggered accessing the `constructor` property.
Backport-PR-URL: #31431
PR-URL: #30343
Fixes: #30333
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>1 parent ea3d4e8 commit a1ce776
2 files changed
Lines changed: 94 additions & 22 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1645,6 +1645,30 @@ function format(...args) { | |||
| 1645 | 1645 | return formatWithOptions(undefined, ...args); | |
| 1646 | 1646 | } | |
| 1647 | 1647 | ||
| 1648 | + function hasBuiltInToString(value) { | ||
| 1649 | + // Count objects that have no `toString` function as built-in. | ||
| 1650 | + if (typeof value.toString !== 'function') { | ||
| 1651 | + return true; | ||
| 1652 | + } | ||
| 1653 | + | ||
| 1654 | + // The object has a own `toString` property. Thus it's not not a built-in one. | ||
| 1655 | + if (ObjectPrototypeHasOwnProperty(value, 'toString')) { | ||
| 1656 | + return false; | ||
| 1657 | + } | ||
| 1658 | + | ||
| 1659 | + // Find the object that has the `toString` property as own property in the | ||
| 1660 | + // prototype chain. | ||
| 1661 | + let pointer = value; | ||
| 1662 | + do { | ||
| 1663 | + pointer = ObjectGetPrototypeOf(pointer); | ||
| 1664 | + } while (!ObjectPrototypeHasOwnProperty(pointer, 'toString')); | ||
| 1665 | + | ||
| 1666 | + // Check closer if the object is a built-in. | ||
| 1667 | + const descriptor = ObjectGetOwnPropertyDescriptor(pointer, 'constructor'); | ||
| 1668 | + return descriptor !== undefined && | ||
| 1669 | + typeof descriptor.value === 'function' && | ||
| 1670 | + builtInObjects.has(descriptor.value.name); | ||
| 1671 | + } | ||
| 1648 | 1672 | ||
| 1649 | 1673 | const firstErrorLine = (error) => error.message.split('\n')[0]; | |
| 1650 | 1674 | let CIRCULAR_ERROR_MESSAGE; | |
@@ -1692,29 +1716,17 @@ function formatWithOptions(inspectOptions, ...args) { | |||
| 1692 | 1716 | tempStr = formatNumber(stylizeNoColor, tempArg); | |
| 1693 | 1717 | } else if (typeof tempArg === 'bigint') { | |
| 1694 | 1718 | tempStr = `${tempArg}n`; | |
| 1719 | + } else if (typeof tempArg !== 'object' || | ||
| 1720 | + tempArg === null || | ||
| 1721 | + !hasBuiltInToString(tempArg)) { | ||
| 1722 | + tempStr = String(tempArg); | ||
| 1695 | 1723 | } else { | |
| 1696 | - let constr; | ||
| 1697 | - if (typeof tempArg !== 'object' || | ||
| 1698 | - tempArg === null || | ||
| 1699 | - (typeof tempArg.toString === 'function' && | ||
| 1700 | - // A direct own property. | ||
| 1701 | - (ObjectPrototypeHasOwnProperty(tempArg, 'toString') || | ||
| 1702 | - // A direct own property on the constructor prototype in | ||
| 1703 | - // case the constructor is not an built-in object. | ||
| 1704 | - ((constr = tempArg.constructor) && | ||
| 1705 | - !builtInObjects.has(constr.name) && | ||
| 1706 | - constr.prototype && | ||
| 1707 | - ObjectPrototypeHasOwnProperty(constr.prototype, | ||
| 1708 | - 'toString'))))) { | ||
| 1709 | - tempStr = String(tempArg); | ||
| 1710 | - } else { | ||
| 1711 | - tempStr = inspect(tempArg, { | ||
| 1712 | - ...inspectOptions, | ||
| 1713 | - compact: 3, | ||
| 1714 | - colors: false, | ||
| 1715 | - depth: 0 | ||
| 1716 | - }); | ||
| 1717 | - } | ||
| 1724 | + tempStr = inspect(tempArg, { | ||
| 1725 | + ...inspectOptions, | ||
| 1726 | + compact: 3, | ||
| 1727 | + colors: false, | ||
| 1728 | + depth: 0 | ||
| 1729 | + }); | ||
| 1718 | 1730 | } | |
| 1719 | 1731 | break; | |
| 1720 | 1732 | case 106: // 'j' | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -160,6 +160,66 @@ assert.strictEqual(util.format('%s', () => 5), '() => 5'); | |||
| 160 | 160 | util.format('%s', new Foobar(5)), | |
| 161 | 161 | 'Foobar [ <5 empty items>, aaa: true ]' | |
| 162 | 162 | ); | |
| 163 | + | ||
| 164 | + // Subclassing: | ||
| 165 | + class B extends Foo {} | ||
| 166 | + | ||
| 167 | + function C() {} | ||
| 168 | + C.prototype.toString = function() { | ||
| 169 | + return 'Custom'; | ||
| 170 | + }; | ||
| 171 | + | ||
| 172 | + function D() { | ||
| 173 | + C.call(this); | ||
| 174 | + } | ||
| 175 | + D.prototype = Object.create(C.prototype); | ||
| 176 | + | ||
| 177 | + assert.strictEqual( | ||
| 178 | + util.format('%s', new B()), | ||
| 179 | + 'Bar' | ||
| 180 | + ); | ||
| 181 | + assert.strictEqual( | ||
| 182 | + util.format('%s', new C()), | ||
| 183 | + 'Custom' | ||
| 184 | + ); | ||
| 185 | + assert.strictEqual( | ||
| 186 | + util.format('%s', new D()), | ||
| 187 | + 'Custom' | ||
| 188 | + ); | ||
| 189 | + | ||
| 190 | + D.prototype.constructor = D; | ||
| 191 | + assert.strictEqual( | ||
| 192 | + util.format('%s', new D()), | ||
| 193 | + 'Custom' | ||
| 194 | + ); | ||
| 195 | + | ||
| 196 | + D.prototype.constructor = null; | ||
| 197 | + assert.strictEqual( | ||
| 198 | + util.format('%s', new D()), | ||
| 199 | + 'Custom' | ||
| 200 | + ); | ||
| 201 | + | ||
| 202 | + D.prototype.constructor = { name: 'Foobar' }; | ||
| 203 | + assert.strictEqual( | ||
| 204 | + util.format('%s', new D()), | ||
| 205 | + 'Custom' | ||
| 206 | + ); | ||
| 207 | + | ||
| 208 | + Object.defineProperty(D.prototype, 'constructor', { | ||
| 209 | + get() { | ||
| 210 | + throw new Error(); | ||
| 211 | + }, | ||
| 212 | + configurable: true | ||
| 213 | + }); | ||
| 214 | + assert.strictEqual( | ||
| 215 | + util.format('%s', new D()), | ||
| 216 | + 'Custom' | ||
| 217 | + ); | ||
| 218 | + | ||
| 219 | + assert.strictEqual( | ||
| 220 | + util.format('%s', Object.create(null)), | ||
| 221 | + '[Object: null prototype] {}' | ||
| 222 | + ); | ||
| 163 | 223 | } | |
| 164 | 224 | ||
| 165 | 225 | // JSON format specifier | |
0 commit comments