Skip to content

Commit ff40068

Browse files
committed
test_runner: do not crash on stdout that mimics a v8 frame
The child test process sends framed report messages and raw user stdout over one pipe, using the bytes FF 0F to mark the start of a frame. User output can contain those same bytes, so #processRawBuffer could read a plausible size from stray stdout and hand the bytes to the v8 deserializer. The deserializer then threw. Because the call had no error handling, the exception aborted the whole test run. Read the frame before advancing the buffer and wrap the deserialize in a try/catch. When the read fails, leave the buffer untouched and stop parsing frames so #drainRawBuffer emits the stray byte as stdout and rescans for the next real header. This turns a fatal crash into recoverable stdout and preserves any real frames that follow the stray bytes. Fixes: #66164 Signed-off-by: Muhammad Faizan Uddin <faizan.uddin94@gmail.com>
1 parent a3bb551 commit ff40068

2 files changed

Lines changed: 72 additions & 2 deletions

File tree

‎lib/internal/test_runner/runner.js‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -512,12 +512,25 @@ class FileTest extends Test {
512512
TypedArrayPrototypeSubarray(concatenatedBuffer, kSerializedSizeHeader, fullMessageSize),
513513
);
514514

515+
let item;
516+
try {
517+
deserializer.readHeader();
518+
item = deserializer.readValue();
519+
} catch {
520+
// The bytes begin with the v8 header magic and a plausible size, but
521+
// the payload is not a real serialized message. This happens when a
522+
// test writes raw bytes to stdout that look like a frame. Leave the
523+
// buffer untouched and stop parsing frames here. #drainRawBuffer then
524+
// emits the stray byte as stdout and rescans for the next real header.
525+
break;
526+
}
527+
528+
// Only advance past the frame once it has been read successfully, so a
529+
// failed read above cannot drop real frames that follow the stray bytes.
515530
bufferHead = TypedArrayPrototypeSubarray(concatenatedBuffer, fullMessageSize);
516531
this.#rawBufferSize = TypedArrayPrototypeGetLength(bufferHead);
517532
this.#rawBuffer = this.#rawBufferSize !== 0 ? [bufferHead] : [];
518533

519-
deserializer.readHeader();
520-
const item = deserializer.readValue();
521534
this.addToReport(item);
522535
}
523536
}

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,17 @@ const oversizedLengthStdout = String.fromCharCode(oversizedLengthHeader[0]) +
3939
Buffer.from(oversizedLengthHeader.subarray(1)).toString('utf-8');
4040
const unsignedOversizedLengthStdout = String.fromCharCode(unsignedOversizedLengthHeader[0]) +
4141
Buffer.from(unsignedOversizedLengthHeader.subarray(1)).toString('utf-8');
42+
// FF 0F followed by a small, plausible size (8) and 8 payload bytes. Unlike the
43+
// oversized headers above, this passes the size check and reaches the
44+
// deserializer, which throws because the payload is not a real message.
45+
// Regression fixture for https://github.com/nodejs/node/issues/66164
46+
const plausibleSizeFalseHeader = Buffer.from([
47+
0xff, 0x0f, // v8 serializer header magic
48+
0x00, 0x00, 0x00, 0x08, // payload size of 8 bytes
49+
0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, // "ABCDEFGH", not a real payload
50+
]);
51+
const plausibleSizeFalseHeaderStdout = String.fromCharCode(plausibleSizeFalseHeader[0]) +
52+
Buffer.from(plausibleSizeFalseHeader.subarray(1)).toString('utf-8');
4253

4354
function collectStdout(reported) {
4455
return reported
@@ -169,6 +180,52 @@ describe('v8 deserializer', common.mustCall(() => {
169180
assert.strictEqual(collectStdout(reported), oversizedLengthStdout);
170181
});
171182

183+
it('should not crash when stdout mimics a v8 frame with a plausible size', async () => {
184+
// Regression test for https://github.com/nodejs/node/issues/66164
185+
// The bytes reach the deserializer and it throws. The parser must emit
186+
// them as stdout instead of letting the error abort the whole run.
187+
const reported = await collectReported([plausibleSizeFalseHeader]);
188+
assert(reported.every((event) => event.type === 'test:stdout'));
189+
assert.strictEqual(collectStdout(reported), plausibleSizeFalseHeaderStdout);
190+
});
191+
192+
it('should resync and parse a real message after a plausible-size false frame', async () => {
193+
// The poison bytes followed by a real serialized message. The parser must
194+
// recover from the failed deserialize and still report the real event.
195+
const reported = await collectReported([
196+
plausibleSizeFalseHeader,
197+
...chunks,
198+
]);
199+
assert.deepStrictEqual(reported.at(-1), reportedDiagnosticEvent);
200+
assert.strictEqual(reported.filter((event) => event.type === 'test:diagnostic').length, 1);
201+
assert.strictEqual(collectStdout(reported), plausibleSizeFalseHeaderStdout);
202+
});
203+
204+
it('should preserve real messages on both sides of a plausible-size false frame', async () => {
205+
// A real message, then the poison bytes, then another real message. Both
206+
// real messages must survive and the poison bytes must become stdout.
207+
const reported = await collectReported([
208+
...chunks,
209+
plausibleSizeFalseHeader,
210+
...chunks,
211+
]);
212+
const diagnostics = reported.filter((event) => event.type === 'test:diagnostic');
213+
assert.strictEqual(diagnostics.length, 2);
214+
diagnostics.forEach((event) => assert.deepStrictEqual(event, reportedDiagnosticEvent));
215+
assert.strictEqual(collectStdout(reported), plausibleSizeFalseHeaderStdout);
216+
});
217+
218+
it('should recover from a plausible-size false frame split across chunks', async () => {
219+
// The same poison bytes arriving in two chunks must still be treated as
220+
// stdout without crashing.
221+
const reported = await collectReported([
222+
plausibleSizeFalseHeader.subarray(0, 3),
223+
plausibleSizeFalseHeader.subarray(3),
224+
]);
225+
assert(reported.every((event) => event.type === 'test:stdout'));
226+
assert.strictEqual(collectStdout(reported), plausibleSizeFalseHeaderStdout);
227+
});
228+
172229
const headerPosition = headerLength * 2 + 4;
173230
for (let i = 0; i < headerPosition + 5; i++) {
174231
const message = `should deserialize a serialized message split into two chunks {...${i},${i + 1}...}`;

0 commit comments

Comments
 (0)