Skip to content

Commit a7cbb90

Browse files
authored
lib: avoid StackOverflow on serializeError
`serializeError` should avoid StackOverflow and the test should not rely on `--stack-size`. PR-URL: nodejs#58075 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
1 parent 50cfc6c commit a7cbb90

File tree

2 files changed

+25
-9
lines changed

2 files changed

+25
-9
lines changed

lib/internal/error_serdes.js

+17-5
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const kSerializedObject = 1;
3636
const kInspectedError = 2;
3737
const kInspectedSymbol = 3;
3838
const kCustomInspectedObject = 4;
39+
const kCircularReference = 5;
3940

4041
const kSymbolStringLength = 'Symbol('.length;
4142

@@ -44,12 +45,12 @@ const errors = {
4445
};
4546
const errorConstructorNames = new SafeSet(ObjectKeys(errors));
4647

47-
function TryGetAllProperties(object, target = object) {
48+
function TryGetAllProperties(object, target = object, rememberSet) {
4849
const all = { __proto__: null };
4950
if (object === null)
5051
return all;
5152
ObjectAssign(all,
52-
TryGetAllProperties(ObjectGetPrototypeOf(object), target));
53+
TryGetAllProperties(ObjectGetPrototypeOf(object), target, rememberSet));
5354
const keys = ObjectGetOwnPropertyNames(object);
5455
ArrayPrototypeForEach(keys, (key) => {
5556
let descriptor;
@@ -68,7 +69,7 @@ function TryGetAllProperties(object, target = object) {
6869
}
6970
}
7071
if (key === 'cause') {
71-
descriptor.value = serializeError(descriptor.value);
72+
descriptor.value = serializeError(descriptor.value, rememberSet);
7273
all[key] = descriptor;
7374
} else if ('value' in descriptor &&
7475
typeof descriptor.value !== 'function' && typeof descriptor.value !== 'symbol') {
@@ -108,21 +109,27 @@ function inspect(...args) {
108109
}
109110

110111
let serialize;
111-
function serializeError(error) {
112+
function serializeError(error, rememberSet = new SafeSet()) {
112113
serialize ??= require('v8').serialize;
113114
if (typeof error === 'symbol') {
114115
return Buffer.from(StringFromCharCode(kInspectedSymbol) + inspect(error), 'utf8');
115116
}
117+
116118
try {
117119
if (typeof error === 'object' &&
118120
ObjectPrototypeToString(error) === '[object Error]') {
121+
if (rememberSet.has(error)) {
122+
return Buffer.from([kCircularReference]);
123+
}
124+
rememberSet.add(error);
125+
119126
const constructors = GetConstructors(error);
120127
for (let i = 0; i < constructors.length; i++) {
121128
const name = GetName(constructors[i]);
122129
if (errorConstructorNames.has(name)) {
123130
const serialized = serialize({
124131
constructor: name,
125-
properties: TryGetAllProperties(error),
132+
properties: TryGetAllProperties(error, error, rememberSet),
126133
});
127134
return Buffer.concat([Buffer.from([kSerializedError]), serialized]);
128135
}
@@ -183,6 +190,11 @@ function deserializeError(error) {
183190
__proto__: null,
184191
[customInspectSymbol]: () => fromBuffer(error).toString('utf8'),
185192
};
193+
case kCircularReference:
194+
return {
195+
__proto__: null,
196+
[customInspectSymbol]: () => '[Circular object]',
197+
};
186198
}
187199
require('assert').fail('This should not happen');
188200
}

test/sequential/test-error-serdes.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --expose-internals --stack-size=64
1+
// Flags: --expose-internals
22
'use strict';
33
require('../common');
44
const assert = require('assert');
@@ -59,7 +59,7 @@ class ErrorWithThowingCause extends Error {
5959
}
6060
class ErrorWithCyclicCause extends Error {
6161
get cause() {
62-
return new ErrorWithCyclicCause();
62+
return this;
6363
}
6464
}
6565
const errorWithCause = Object
@@ -83,14 +83,18 @@ assert.strictEqual(Object.hasOwn(cycle(errorWithCyclicCause), 'cause'), true);
8383
assert.deepStrictEqual(cycle(new ErrorWithCause('Error with cause')).cause, new Error('err'));
8484
assert.strictEqual(cycle(new ErrorWithThowingCause('Error with cause')).cause, undefined);
8585
assert.strictEqual(Object.hasOwn(cycle(new ErrorWithThowingCause('Error with cause')), 'cause'), false);
86-
// When the cause is cyclic, it is serialized until Maximum call stack size is reached
86+
// When the cause is cyclic, it is serialized as a dumb circular reference object.
8787
let depth = 0;
8888
let e = cycle(new ErrorWithCyclicCause('Error with cause'));
8989
while (e.cause) {
9090
e = e.cause;
9191
depth++;
9292
}
93-
assert(depth > 1);
93+
assert.strictEqual(depth, 1);
94+
assert.strictEqual(
95+
inspect(cycle(new ErrorWithCyclicCause('Error with cause')).cause),
96+
'[Circular object]',
97+
);
9498

9599

96100
{

0 commit comments

Comments
 (0)