Skip to content

Commit 83054e8

Browse files
thisalihassanaduh95
authored andcommitted
test_runner: avoid hanging on incomplete v8 frames
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com> PR-URL: #62704 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent e49154f commit 83054e8

2 files changed

Lines changed: 124 additions & 18 deletions

File tree

lib/internal/test_runner/runner.js

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const {
2525
SafePromiseAllSettledReturnVoid,
2626
SafeSet,
2727
String,
28+
StringFromCharCode,
2829
StringPrototypeIndexOf,
2930
StringPrototypeSlice,
3031
StringPrototypeStartsWith,
@@ -261,6 +262,7 @@ class FileTest extends Test {
261262
#rawBuffer = []; // Raw data waiting to be parsed
262263
#rawBufferSize = 0;
263264
#reportedChildren = 0;
265+
#pendingPartialV8Header = false;
264266
failedSubtests = false;
265267

266268
constructor(options) {
@@ -352,6 +354,12 @@ class FileTest extends Test {
352354
}
353355
parseMessage(readData) {
354356
let dataLength = TypedArrayPrototypeGetLength(readData);
357+
if (this.#pendingPartialV8Header) {
358+
readData = Buffer.concat([TypedArrayPrototypeSubarray(v8Header, 0, 1), readData]);
359+
dataLength = TypedArrayPrototypeGetLength(readData);
360+
this.#pendingPartialV8Header = false;
361+
}
362+
355363
if (dataLength === 0) return;
356364
const partialV8Header = readData[dataLength - 1] === v8Header[0];
357365

@@ -362,22 +370,52 @@ class FileTest extends Test {
362370
dataLength--;
363371
}
364372

365-
if (this.#rawBuffer[0] && TypedArrayPrototypeGetLength(this.#rawBuffer[0]) < kSerializedSizeHeader) {
366-
this.#rawBuffer[0] = Buffer.concat([this.#rawBuffer[0], readData]);
367-
} else {
368-
ArrayPrototypePush(this.#rawBuffer, readData);
373+
if (dataLength > 0) {
374+
if (this.#rawBuffer[0] && TypedArrayPrototypeGetLength(this.#rawBuffer[0]) < kSerializedSizeHeader) {
375+
this.#rawBuffer[0] = Buffer.concat([this.#rawBuffer[0], readData]);
376+
} else {
377+
ArrayPrototypePush(this.#rawBuffer, readData);
378+
}
379+
this.#rawBufferSize += dataLength;
380+
this.#processRawBuffer();
369381
}
370-
this.#rawBufferSize += dataLength;
371-
this.#processRawBuffer();
372382

373383
if (partialV8Header) {
374-
ArrayPrototypePush(this.#rawBuffer, TypedArrayPrototypeSubarray(v8Header, 0, 1));
375-
this.#rawBufferSize++;
384+
this.#pendingPartialV8Header = true;
376385
}
377386
}
378387
#drainRawBuffer() {
388+
if (this.#pendingPartialV8Header) {
389+
ArrayPrototypePush(this.#rawBuffer, TypedArrayPrototypeSubarray(v8Header, 0, 1));
390+
this.#rawBufferSize++;
391+
this.#pendingPartialV8Header = false;
392+
}
393+
379394
while (this.#rawBuffer.length > 0) {
395+
const prevBufferLength = this.#rawBuffer.length;
396+
const prevBufferSize = this.#rawBufferSize;
380397
this.#processRawBuffer();
398+
399+
if (this.#rawBuffer.length === prevBufferLength &&
400+
this.#rawBufferSize === prevBufferSize) {
401+
const bufferHead = this.#rawBuffer[0];
402+
this.addToReport({
403+
__proto__: null,
404+
type: 'test:stdout',
405+
data: {
406+
__proto__: null,
407+
file: this.name,
408+
message: StringFromCharCode(bufferHead[0]),
409+
},
410+
});
411+
412+
if (TypedArrayPrototypeGetLength(bufferHead) === 1) {
413+
ArrayPrototypeShift(this.#rawBuffer);
414+
} else {
415+
this.#rawBuffer[0] = TypedArrayPrototypeSubarray(bufferHead, 1);
416+
}
417+
this.#rawBufferSize--;
418+
}
381419
}
382420
}
383421
#processRawBuffer() {

test/parallel/test-runner-v8-deserializer.mjs

Lines changed: 78 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,29 @@ async function toArray(chunks) {
1414
return arr;
1515
}
1616

17-
const chunks = await toArray(serializer([
18-
{ type: 'test:diagnostic', data: { nesting: 0, details: {}, message: 'diagnostic' } },
19-
]));
17+
const diagnosticEvent = {
18+
type: 'test:diagnostic',
19+
data: { nesting: 0, details: {}, message: 'diagnostic' },
20+
};
21+
const chunks = await toArray(serializer([diagnosticEvent]));
2022
const defaultSerializer = new DefaultSerializer();
2123
defaultSerializer.writeHeader();
2224
const headerLength = defaultSerializer.releaseBuffer().length;
25+
const headerOnly = Buffer.from([0xff, 0x0f]);
26+
const oversizedLengthHeader = Buffer.from([0xff, 0x0f, 0x7f, 0xff, 0xff, 0xff]);
27+
const truncatedLengthHeader = Buffer.from([0xff, 0x0f, 0x00, 0x01, 0x00, 0x00]);
28+
// Expected stdout for oversizedLengthHeader: first byte is emitted via
29+
// String.fromCharCode (byte-by-byte fallback in #drainRawBuffer), remaining
30+
// bytes go through the nonSerialized UTF-8 decode path in #processRawBuffer.
31+
const oversizedLengthStdout = String.fromCharCode(oversizedLengthHeader[0]) +
32+
Buffer.from(oversizedLengthHeader.subarray(1)).toString('utf-8');
33+
34+
function collectStdout(reported) {
35+
return reported
36+
.filter((event) => event.type === 'test:stdout')
37+
.map((event) => event.data.message)
38+
.join('');
39+
}
2340

2441
describe('v8 deserializer', common.mustCall(() => {
2542
let fileTest;
@@ -56,35 +73,86 @@ describe('v8 deserializer', common.mustCall(() => {
5673

5774
it('should deserialize a serialized chunk', async () => {
5875
const reported = await collectReported(chunks);
59-
assert.deepStrictEqual(reported, [
60-
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
61-
]);
76+
assert.deepStrictEqual(reported, [diagnosticEvent]);
6277
});
6378

6479
it('should deserialize a serialized chunk after non-serialized chunk', async () => {
6580
const reported = await collectReported([Buffer.concat([Buffer.from('unknown'), ...chunks])]);
6681
assert.deepStrictEqual(reported, [
6782
{ data: { __proto__: null, file: 'filetest', message: 'unknown' }, type: 'test:stdout' },
68-
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
83+
diagnosticEvent,
6984
]);
7085
});
7186

7287
it('should deserialize a serialized chunk before non-serialized output', async () => {
7388
const reported = await collectReported([Buffer.concat([ ...chunks, Buffer.from('unknown')])]);
7489
assert.deepStrictEqual(reported, [
75-
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
90+
diagnosticEvent,
7691
{ data: { __proto__: null, file: 'filetest', message: 'unknown' }, type: 'test:stdout' },
7792
]);
7893
});
7994

95+
it('should not hang when buffer starts with v8Header followed by oversized length', async () => {
96+
// Regression test for https://github.com/nodejs/node/issues/62693
97+
// FF 0F is the v8 serializer header; the next 4 bytes are read as a
98+
// big-endian message size. 0x7FFFFFFF far exceeds any actual buffer
99+
// size, causing #processRawBuffer to make no progress and
100+
// #drainRawBuffer to loop forever without the no-progress guard.
101+
const reported = await collectReported([oversizedLengthHeader]);
102+
assert.partialDeepStrictEqual(
103+
reported,
104+
Array.from({ length: reported.length }, () => ({ type: 'test:stdout' })),
105+
);
106+
assert.strictEqual(collectStdout(reported), oversizedLengthStdout);
107+
});
108+
109+
it('should flush incomplete v8 frame as stdout and keep prior valid data', async () => {
110+
// A valid non-serialized message followed by bytes that look like
111+
// a v8 header with a truncated/oversized length.
112+
const reported = await collectReported([
113+
Buffer.from('hello'),
114+
truncatedLengthHeader,
115+
]);
116+
assert.strictEqual(collectStdout(reported), `hello${truncatedLengthHeader.toString('latin1')}`);
117+
});
118+
119+
it('should flush v8Header-only bytes as stdout when stream ends', async () => {
120+
// Just the two-byte v8 header with no size field at all.
121+
const reported = await collectReported([headerOnly]);
122+
assert(reported.every((event) => event.type === 'test:stdout'));
123+
assert.strictEqual(collectStdout(reported), headerOnly.toString('latin1'));
124+
});
125+
126+
it('should resync and parse valid messages after false v8 header', async () => {
127+
// A false v8 header (FF 0F + oversized length) followed by a
128+
// legitimate serialized message. The parser must skip the corrupt
129+
// bytes and still deserialize the real message.
130+
const reported = await collectReported([
131+
oversizedLengthHeader,
132+
...chunks,
133+
]);
134+
assert.deepStrictEqual(reported.at(-1), diagnosticEvent);
135+
assert.strictEqual(reported.filter((event) => event.type === 'test:diagnostic').length, 1);
136+
assert.strictEqual(collectStdout(reported), oversizedLengthStdout);
137+
});
138+
139+
it('should preserve a false v8 header split across chunks', async () => {
140+
const reported = await collectReported([
141+
oversizedLengthHeader.subarray(0, 1),
142+
oversizedLengthHeader.subarray(1),
143+
]);
144+
assert(reported.every((event) => event.type === 'test:stdout'));
145+
assert.strictEqual(collectStdout(reported), oversizedLengthStdout);
146+
});
147+
80148
const headerPosition = headerLength * 2 + 4;
81149
for (let i = 0; i < headerPosition + 5; i++) {
82150
const message = `should deserialize a serialized message split into two chunks {...${i},${i + 1}...}`;
83151
it(message, async () => {
84152
const data = chunks[0];
85153
const reported = await collectReported([data.subarray(0, i), data.subarray(i)]);
86154
assert.deepStrictEqual(reported, [
87-
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
155+
diagnosticEvent,
88156
]);
89157
});
90158

@@ -96,7 +164,7 @@ describe('v8 deserializer', common.mustCall(() => {
96164
]);
97165
assert.deepStrictEqual(reported, [
98166
{ data: { __proto__: null, file: 'filetest', message: 'unknown' }, type: 'test:stdout' },
99-
{ data: { nesting: 0, details: {}, message: 'diagnostic' }, type: 'test:diagnostic' },
167+
diagnosticEvent,
100168
{ data: { __proto__: null, file: 'filetest', message: 'unknown' }, type: 'test:stdout' },
101169
]);
102170
}

0 commit comments

Comments
 (0)