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 6a0dd1c

Browse filesBrowse files
BridgeARtargos
BridgeAR
authored and
targos
committed
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. 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 1040e72 commit 6a0dd1c
Copy full SHA for 6a0dd1c

File tree

Expand file treeCollapse file tree

2 files changed

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

2 files changed

+95
-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
+35-22Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,6 +1567,31 @@ function reduceToSingleString(
15671567
return `${braces[0]}${ln}${join(output, `,\n${indentation} `)} ${braces[1]}`;
15681568
}
15691569

1570+
function hasBuiltInToString(value) {
1571+
// Count objects that have no `toString` function as built-in.
1572+
if (typeof value.toString !== 'function') {
1573+
return true;
1574+
}
1575+
1576+
// The object has a own `toString` property. Thus it's not not a built-in one.
1577+
if (ObjectPrototypeHasOwnProperty(value, 'toString')) {
1578+
return false;
1579+
}
1580+
1581+
// Find the object that has the `toString` property as own property in the
1582+
// prototype chain.
1583+
let pointer = value;
1584+
do {
1585+
pointer = ObjectGetPrototypeOf(pointer);
1586+
} while (!ObjectPrototypeHasOwnProperty(pointer, 'toString'));
1587+
1588+
// Check closer if the object is a built-in.
1589+
const descriptor = ObjectGetOwnPropertyDescriptor(pointer, 'constructor');
1590+
return descriptor !== undefined &&
1591+
typeof descriptor.value === 'function' &&
1592+
builtInObjects.has(descriptor.value.name);
1593+
}
1594+
15701595
const firstErrorLine = (error) => error.message.split('\n')[0];
15711596
let CIRCULAR_ERROR_MESSAGE;
15721597
function tryStringify(arg) {
@@ -1625,29 +1650,17 @@ function formatWithOptionsInternal(inspectOptions, ...args) {
16251650
tempStr = formatNumber(stylizeNoColor, tempArg);
16261651
} else if (typeof tempArg === 'bigint') {
16271652
tempStr = `${tempArg}n`;
1653+
} else if (typeof tempArg !== 'object' ||
1654+
tempArg === null ||
1655+
!hasBuiltInToString(tempArg)) {
1656+
tempStr = String(tempArg);
16281657
} else {
1629-
let constr;
1630-
if (typeof tempArg !== 'object' ||
1631-
tempArg === null ||
1632-
(typeof tempArg.toString === 'function' &&
1633-
// A direct own property.
1634-
(ObjectPrototypeHasOwnProperty(tempArg, 'toString') ||
1635-
// A direct own property on the constructor prototype in
1636-
// case the constructor is not an built-in object.
1637-
((constr = tempArg.constructor) &&
1638-
!builtInObjects.has(constr.name) &&
1639-
constr.prototype &&
1640-
ObjectPrototypeHasOwnProperty(constr.prototype,
1641-
'toString'))))) {
1642-
tempStr = String(tempArg);
1643-
} else {
1644-
tempStr = inspect(tempArg, {
1645-
...inspectOptions,
1646-
compact: 3,
1647-
colors: false,
1648-
depth: 0
1649-
});
1650-
}
1658+
tempStr = inspect(tempArg, {
1659+
...inspectOptions,
1660+
compact: 3,
1661+
colors: false,
1662+
depth: 0
1663+
});
16511664
}
16521665
break;
16531666
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.