Skip to content

Commit 398674a

Browse files
legendecasRafaelGSS
authored andcommitted
lib: avoid StackOverflow on serializeError
`serializeError` should avoid StackOverflow and the test should not rely on `--stack-size`. PR-URL: #58075 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
1 parent c4ca0d7 commit 398674a

2 files changed

Lines changed: 25 additions & 9 deletions

File tree

‎lib/internal/error_serdes.js‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const kSerializedObject = 1;
3636
constkInspectedError=2;
3737
constkInspectedSymbol=3;
3838
constkCustomInspectedObject=4;
39+
constkCircularReference=5;
3940

4041
constkSymbolStringLength='Symbol('.length;
4142

@@ -44,12 +45,12 @@ const errors = {
4445
};
4546
consterrorConstructorNames=newSafeSet(ObjectKeys(errors));
4647

47-
functionTryGetAllProperties(object,target=object){
48+
functionTryGetAllProperties(object,target=object,rememberSet){
4849
constall={__proto__: null};
4950
if(object===null)
5051
returnall;
5152
ObjectAssign(all,
52-
TryGetAllProperties(ObjectGetPrototypeOf(object),target));
53+
TryGetAllProperties(ObjectGetPrototypeOf(object),target,rememberSet));
5354
constkeys=ObjectGetOwnPropertyNames(object);
5455
ArrayPrototypeForEach(keys,(key)=>{
5556
letdescriptor;
@@ -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
}elseif('value'indescriptor&&
7475
typeofdescriptor.value!=='function'&&typeofdescriptor.value!=='symbol'){
@@ -108,21 +109,27 @@ function inspect(...args) {
108109
}
109110

110111
letserialize;
111-
functionserializeError(error){
112+
functionserializeError(error,rememberSet=newSafeSet()){
112113
serialize??=require('v8').serialize;
113114
if(typeoferror==='symbol'){
114115
returnBuffer.from(StringFromCharCode(kInspectedSymbol)+inspect(error),'utf8');
115116
}
117+
116118
try{
117119
if(typeoferror==='object'&&
118120
ObjectPrototypeToString(error)==='[object Error]'){
121+
if(rememberSet.has(error)){
122+
returnBuffer.from([kCircularReference]);
123+
}
124+
rememberSet.add(error);
125+
119126
constconstructors=GetConstructors(error);
120127
for(leti=0;i<constructors.length;i++){
121128
constname=GetName(constructors[i]);
122129
if(errorConstructorNames.has(name)){
123130
constserialized=serialize({
124131
constructor: name,
125-
properties: TryGetAllProperties(error),
132+
properties: TryGetAllProperties(error,error,rememberSet),
126133
});
127134
returnBuffer.concat([Buffer.from([kSerializedError]),serialized]);
128135
}
@@ -183,6 +190,11 @@ function deserializeError(error) {
183190
__proto__: null,
184191
[customInspectSymbol]: ()=>fromBuffer(error).toString('utf8'),
185192
};
193+
casekCircularReference:
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‎

Lines changed: 8 additions & 4 deletions
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
constassert=require('assert');
@@ -59,7 +59,7 @@ class ErrorWithThowingCause extends Error {
5959
}
6060
classErrorWithCyclicCauseextendsError{
6161
getcause(){
62-
returnnewErrorWithCyclicCause();
62+
returnthis;
6363
}
6464
}
6565
consterrorWithCause=Object
@@ -83,14 +83,18 @@ assert.strictEqual(Object.hasOwn(cycle(errorWithCyclicCause), 'cause'), true);
8383
assert.deepStrictEqual(cycle(newErrorWithCause('Error with cause')).cause,newError('err'));
8484
assert.strictEqual(cycle(newErrorWithThowingCause('Error with cause')).cause,undefined);
8585
assert.strictEqual(Object.hasOwn(cycle(newErrorWithThowingCause('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
letdepth=0;
8888
lete=cycle(newErrorWithCyclicCause('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(newErrorWithCyclicCause('Error with cause')).cause),
96+
'[Circular object]',
97+
);
9498

9599

96100
{

0 commit comments

Comments
 (0)