Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit a1ce776

Browse filesBrowse files
BridgeARMylesBorins
authored andcommitted
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
Copy full SHA for a1ce776

File tree

Expand file treeCollapse file tree

2 files changed

+94
-22
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+94
-22
lines changed
Open diff view settings
Collapse file

‎lib/internal/util/inspect.js‎

Copy file name to clipboardExpand all lines: lib/internal/util/inspect.js
+34-22Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1645,6 +1645,30 @@ function format(...args) {
16451645
return formatWithOptions(undefined, ...args);
16461646
}
16471647

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+
}
16481672

16491673
const firstErrorLine = (error) => error.message.split('\n')[0];
16501674
let CIRCULAR_ERROR_MESSAGE;
@@ -1692,29 +1716,17 @@ function formatWithOptions(inspectOptions, ...args) {
16921716
tempStr = formatNumber(stylizeNoColor, tempArg);
16931717
} else if (typeof tempArg === 'bigint') {
16941718
tempStr = `${tempArg}n`;
1719+
} else if (typeof tempArg !== 'object' ||
1720+
tempArg === null ||
1721+
!hasBuiltInToString(tempArg)) {
1722+
tempStr = String(tempArg);
16951723
} 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+
});
17181730
}
17191731
break;
17201732
case 106: // 'j'
Collapse file

‎test/parallel/test-util-format.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-util-format.js
+60Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,66 @@ assert.strictEqual(util.format('%s', () => 5), '() => 5');
160160
util.format('%s', new Foobar(5)),
161161
'Foobar [ <5 empty items>, aaa: true ]'
162162
);
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+
);
163223
}
164224

165225
// JSON format specifier

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.