diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 67c78e03c83..18451ade271 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -467,58 +467,97 @@ class FileTest extends Test { #processRawBuffer() { // This method is called when it is known that there is at least one message let bufferHead = this.#rawBuffer[0]; - let headerIndex = bufferHead.indexOf(v8Header); - let nonSerialized = new FastBuffer(); - - while (bufferHead && headerIndex !== 0) { - const nonSerializedData = headerIndex === -1 ? - bufferHead : - bufferHead.slice(0, headerIndex); - nonSerialized = Buffer.concat([nonSerialized, nonSerializedData]); - this.#rawBufferSize -= TypedArrayPrototypeGetLength(nonSerializedData); - if (headerIndex === -1) { - ArrayPrototypeShift(this.#rawBuffer); - } else { - this.#rawBuffer[0] = TypedArrayPrototypeSubarray(bufferHead, headerIndex); + + while (bufferHead) { + // Emit any leading bytes that are not the start of a frame as stdout, + // until a v8 header sits at the front of the buffer. + let headerIndex = bufferHead.indexOf(v8Header); + let nonSerialized = new FastBuffer(); + + while (bufferHead && headerIndex !== 0) { + const nonSerializedData = headerIndex === -1 ? + bufferHead : + bufferHead.slice(0, headerIndex); + nonSerialized = Buffer.concat([nonSerialized, nonSerializedData]); + this.#rawBufferSize -= TypedArrayPrototypeGetLength(nonSerializedData); + if (headerIndex === -1) { + ArrayPrototypeShift(this.#rawBuffer); + } else { + this.#rawBuffer[0] = TypedArrayPrototypeSubarray(bufferHead, headerIndex); + } + bufferHead = this.#rawBuffer[0]; + headerIndex = bufferHead?.indexOf(v8Header); } - bufferHead = this.#rawBuffer[0]; - headerIndex = bufferHead?.indexOf(v8Header); - } - if (TypedArrayPrototypeGetLength(nonSerialized) > 0) { - this.addToReport({ - __proto__: null, - type: 'test:stdout', - data: { __proto__: null, file: this.name, message: nonSerialized.toString('utf-8') }, - }); - } + if (TypedArrayPrototypeGetLength(nonSerialized) > 0) { + this.addToReport({ + __proto__: null, + type: 'test:stdout', + data: { __proto__: null, file: this.name, message: nonSerialized.toString('utf-8') }, + }); + } - while (bufferHead?.length >= kSerializedSizeHeader) { - // We call `readUInt32BE` manually here, because this is faster than first converting - // it to a buffer and using `readUInt32BE` on that. - const fullMessageSize = (( - bufferHead[kV8HeaderLength] << 24 | - bufferHead[kV8HeaderLength + 1] << 16 | - bufferHead[kV8HeaderLength + 2] << 8 | - bufferHead[kV8HeaderLength + 3] - ) >>> 0) + kSerializedSizeHeader; + let mimicsFrame = false; + while (bufferHead?.length >= kSerializedSizeHeader) { + // We call `readUInt32BE` manually here, because this is faster than first converting + // it to a buffer and using `readUInt32BE` on that. + const fullMessageSize = (( + bufferHead[kV8HeaderLength] << 24 | + bufferHead[kV8HeaderLength + 1] << 16 | + bufferHead[kV8HeaderLength + 2] << 8 | + bufferHead[kV8HeaderLength + 3] + ) >>> 0) + kSerializedSizeHeader; + + if (this.#rawBufferSize < fullMessageSize) break; + + const concatenatedBuffer = this.#rawBuffer.length === 1 ? + this.#rawBuffer[0] : Buffer.concat(this.#rawBuffer, this.#rawBufferSize); + + // A real frame repeats the v8 header at the start of its payload, right + // before the serialized value. If that inner header is missing, these + // bytes only mimic a frame, so stop deserializing and fall through to + // the resync below. A genuine frame that fails to deserialize is left + // to throw, so real report-protocol regressions are not hidden. + if (fullMessageSize - kSerializedSizeHeader < kV8HeaderLength || + concatenatedBuffer.indexOf(v8Header, kSerializedSizeHeader) !== kSerializedSizeHeader) { + mimicsFrame = true; + break; + } - if (this.#rawBufferSize < fullMessageSize) break; + const deserializer = new DefaultDeserializer( + TypedArrayPrototypeSubarray(concatenatedBuffer, kSerializedSizeHeader, fullMessageSize), + ); + deserializer.readHeader(); + const item = deserializer.readValue(); - const concatenatedBuffer = this.#rawBuffer.length === 1 ? - this.#rawBuffer[0] : Buffer.concat(this.#rawBuffer, this.#rawBufferSize); + bufferHead = TypedArrayPrototypeSubarray(concatenatedBuffer, fullMessageSize); + this.#rawBufferSize = TypedArrayPrototypeGetLength(bufferHead); + this.#rawBuffer = this.#rawBufferSize !== 0 ? [bufferHead] : []; - const deserializer = new DefaultDeserializer( - TypedArrayPrototypeSubarray(concatenatedBuffer, kSerializedSizeHeader, fullMessageSize), - ); + this.addToReport(item); + } - bufferHead = TypedArrayPrototypeSubarray(concatenatedBuffer, fullMessageSize); - this.#rawBufferSize = TypedArrayPrototypeGetLength(bufferHead); - this.#rawBuffer = this.#rawBufferSize !== 0 ? [bufferHead] : []; + if (!mimicsFrame) { + // The head is either exhausted or an incomplete frame we must wait for. + break; + } - deserializer.readHeader(); - const item = deserializer.readValue(); - this.addToReport(item); + // The head only mimics a frame. Emit its first byte as stdout and restart + // the header search, so a real frame that follows is reported right away + // instead of waiting for #drainRawBuffer at shutdown. This mirrors the no + // progress path in #drainRawBuffer, so the stdout bytes come out the same. + this.addToReport({ + __proto__: null, + type: 'test:stdout', + data: { __proto__: null, file: this.name, message: StringFromCharCode(bufferHead[0]) }, + }); + if (TypedArrayPrototypeGetLength(bufferHead) === 1) { + ArrayPrototypeShift(this.#rawBuffer); + } else { + this.#rawBuffer[0] = TypedArrayPrototypeSubarray(bufferHead, 1); + } + this.#rawBufferSize--; + bufferHead = this.#rawBuffer[0]; } } } diff --git a/test/parallel/test-runner-v8-deserializer.mjs b/test/parallel/test-runner-v8-deserializer.mjs index 3a4db367ca6..38a7feae059 100644 --- a/test/parallel/test-runner-v8-deserializer.mjs +++ b/test/parallel/test-runner-v8-deserializer.mjs @@ -39,6 +39,38 @@ const oversizedLengthStdout = String.fromCharCode(oversizedLengthHeader[0]) + Buffer.from(oversizedLengthHeader.subarray(1)).toString('utf-8'); const unsignedOversizedLengthStdout = String.fromCharCode(unsignedOversizedLengthHeader[0]) + Buffer.from(unsignedOversizedLengthHeader.subarray(1)).toString('utf-8'); +// FF 0F followed by a small, plausible size (8) and 8 payload bytes. Unlike the +// oversized headers above, this passes the size check, but its payload does not +// begin with the inner v8 header a real frame carries, so it is treated as +// stdout instead of reaching the deserializer. +// Regression fixture for https://github.com/nodejs/node/issues/66164 +const plausibleSizeFalseHeader = Buffer.from([ + 0xff, 0x0f, // V8 serializer header magic + 0x00, 0x00, 0x00, 0x08, // Payload size of 8 bytes + 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, // "ABCDEFGH", not a real payload +]); +const plausibleSizeFalseHeaderStdout = String.fromCharCode(plausibleSizeFalseHeader[0]) + + Buffer.from(plausibleSizeFalseHeader.subarray(1)).toString('utf-8'); +// FF 0F, a valid size, then the inner v8 header a real frame repeats, followed +// by a byte that is not a valid serialized value. This passes the inner header +// check and reaches the deserializer, which throws. This is what a genuine +// report-protocol regression looks like, so the parser must let the error +// surface instead of hiding it as stdout. +const headeredCorruptFrame = Buffer.from([ + 0xff, 0x0f, // Outer v8 serializer header magic + 0x00, 0x00, 0x00, 0x03, // Payload size of 3 bytes + 0xff, 0x0f, // Inner v8 header that a real frame repeats + 0xee, // Not a valid serialized value +]); +// FF 0F with a declared size of 1, then more header bytes. The payload is +// shorter than the inner v8 header a real frame carries, so it can never be a +// real frame. The length guard must reject it as stdout without reaching the +// deserializer. +const shortPayloadFalseHeader = Buffer.from([ + 0xff, 0x0f, // Outer v8 serializer header magic + 0x00, 0x00, 0x00, 0x01, // Payload size of 1 byte, too short for a header + 0xff, 0x0f, // Trailing bytes that also look like a header +]); function collectStdout(reported) { return reported @@ -169,6 +201,84 @@ describe('v8 deserializer', common.mustCall(() => { assert.strictEqual(collectStdout(reported), oversizedLengthStdout); }); + it('should not crash when stdout mimics a v8 frame with a plausible size', async () => { + // Regression test for https://github.com/nodejs/node/issues/66164 + // The payload does not start with the inner v8 header that a real frame + // carries, so the parser treats the bytes as stdout instead of handing + // them to the deserializer and aborting the whole run. + const reported = await collectReported([plausibleSizeFalseHeader]); + assert(reported.every((event) => event.type === 'test:stdout')); + assert.strictEqual(collectStdout(reported), plausibleSizeFalseHeaderStdout); + }); + + it('should resync live and report a real message after a false frame', async () => { + // Feed the poison bytes then a real message but never call drain(). Recovery + // must happen live, so the real event is reported right away. If resync only + // ran at shutdown, the diagnostic would still be buffered and missing here. + // The reporter is a stream, so flush it with end() and finished() before + // asserting, rather than reading it synchronously. + fileTest.parseMessage(plausibleSizeFalseHeader); + chunks.forEach((chunk) => fileTest.parseMessage(chunk)); + fileTest.reporter.end(); + await finished(fileTest.reporter); + assert.deepStrictEqual(reported.at(-1), reportedDiagnosticEvent); + assert.strictEqual(reported.filter((event) => event.type === 'test:diagnostic').length, 1); + assert.strictEqual(collectStdout(reported), plausibleSizeFalseHeaderStdout); + }); + + it('should preserve real messages on both sides of a plausible-size false frame', async () => { + // A real message, then the poison bytes, then another real message. Both + // real messages must survive and the poison bytes must become stdout. + const reported = await collectReported([ + ...chunks, + plausibleSizeFalseHeader, + ...chunks, + ]); + const diagnostics = reported.filter((event) => event.type === 'test:diagnostic'); + assert.strictEqual(diagnostics.length, 2); + diagnostics.forEach((event) => assert.deepStrictEqual(event, reportedDiagnosticEvent)); + assert.strictEqual(collectStdout(reported), plausibleSizeFalseHeaderStdout); + }); + + it('should recover from a plausible-size false frame split across chunks', async () => { + // The same poison bytes arriving in two chunks must still be treated as + // stdout without crashing. + const reported = await collectReported([ + plausibleSizeFalseHeader.subarray(0, 3), + plausibleSizeFalseHeader.subarray(3), + ]); + assert(reported.every((event) => event.type === 'test:stdout')); + assert.strictEqual(collectStdout(reported), plausibleSizeFalseHeaderStdout); + }); + + it('should resync through several stray frames in a row', async () => { + // Two false frames back to back in one read, then a real one. The parser + // must peel each stray frame off as stdout and still report the real event. + const reported = await collectReported([ + Buffer.concat([plausibleSizeFalseHeader, plausibleSizeFalseHeader, ...chunks]), + ]); + assert.deepStrictEqual(reported.at(-1), reportedDiagnosticEvent); + assert.strictEqual(reported.filter((event) => event.type === 'test:diagnostic').length, 1); + assert.strictEqual(collectStdout(reported), + plausibleSizeFalseHeaderStdout + plausibleSizeFalseHeaderStdout); + }); + + it('should surface a genuinely corrupt frame instead of hiding it', () => { + // A frame with both v8 headers and a valid size but an invalid value is + // what a real report-protocol regression looks like, not stray stdout. + // The parser must let the deserialize error surface instead of silently + // turning it into stdout. + assert.throws(() => fileTest.parseMessage(headeredCorruptFrame), /deserialize/); + }); + + it('should treat a frame whose payload is shorter than the header as stdout', async () => { + // The declared size is smaller than the inner v8 header, so the length + // guard must reject the bytes as stdout instead of reaching the + // deserializer. + const reported = await collectReported([shortPayloadFalseHeader]); + assert(reported.every((event) => event.type === 'test:stdout')); + }); + const headerPosition = headerLength * 2 + 4; for (let i = 0; i < headerPosition + 5; i++) { const message = `should deserialize a serialized message split into two chunks {...${i},${i + 1}...}`;