diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 67c78e03c831..98947c742b21 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -83,8 +83,6 @@ const { kTestTimeoutFailure, Test, } = require('internal/test_runner/test'); -const { FastBuffer } = require('internal/buffer'); - const { createRandomSeed, convertStringToRegExp, @@ -465,59 +463,87 @@ 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); + // This method is called when it is known that there is at least one message. + // Each pass looks at the head of the buffer and handles exactly one of four + // cases, then re-checks the new head. Recovery from stray stdout that only + // mimics a frame happens inline here, so no separate resync pass is needed. + while (this.#rawBuffer.length > 0) { + const bufferHead = this.#rawBuffer[0]; + const headerIndex = bufferHead.indexOf(v8Header); + + // 1. The head does not start with a frame header. Emit the bytes before + // the next header, or the whole head when there is none, as stdout and + // advance to the next header. + if (headerIndex !== 0) { + const nonSerialized = headerIndex === -1 ? + bufferHead : TypedArrayPrototypeSubarray(bufferHead, 0, headerIndex); + this.addToReport({ + __proto__: null, + type: 'test:stdout', + data: { __proto__: null, file: this.name, message: nonSerialized.toString('utf-8') }, + }); + this.#rawBufferSize -= TypedArrayPrototypeGetLength(nonSerialized); + if (headerIndex === -1) { + ArrayPrototypeShift(this.#rawBuffer); + } else { + this.#rawBuffer[0] = TypedArrayPrototypeSubarray(bufferHead, headerIndex); + } + continue; } - 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') }, - }); - } - 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. + // 2. The head starts with a header but the whole frame has not arrived + // yet. Stop and wait for more data. + if (TypedArrayPrototypeGetLength(bufferHead) < kSerializedSizeHeader) { + break; + } + // 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; + if (this.#rawBufferSize < fullMessageSize) { + break; + } const concatenatedBuffer = this.#rawBuffer.length === 1 ? - this.#rawBuffer[0] : Buffer.concat(this.#rawBuffer, this.#rawBufferSize); + bufferHead : Buffer.concat(this.#rawBuffer, this.#rawBufferSize); + + // 3. The head only mimics a frame. A real frame repeats the v8 header at + // the start of its payload, right before the serialized value, so a + // payload too short for that header, or one that does not start with + // it, is stray stdout. Emit one byte and let the next pass resync on + // the following header. 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) { + 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--; + continue; + } + // 4. A real frame. Deserialize it and continue from the remaining bytes. const deserializer = new DefaultDeserializer( TypedArrayPrototypeSubarray(concatenatedBuffer, kSerializedSizeHeader, fullMessageSize), ); - - bufferHead = TypedArrayPrototypeSubarray(concatenatedBuffer, fullMessageSize); - this.#rawBufferSize = TypedArrayPrototypeGetLength(bufferHead); - this.#rawBuffer = this.#rawBufferSize !== 0 ? [bufferHead] : []; - deserializer.readHeader(); const item = deserializer.readValue(); + + const remaining = TypedArrayPrototypeSubarray(concatenatedBuffer, fullMessageSize); + this.#rawBufferSize = TypedArrayPrototypeGetLength(remaining); + this.#rawBuffer = this.#rawBufferSize !== 0 ? [remaining] : []; + this.addToReport(item); } } diff --git a/test/parallel/test-runner-v8-deserializer.mjs b/test/parallel/test-runner-v8-deserializer.mjs index 3a4db367ca6d..38a7feae0599 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}...}`;