nodejs/node · #66273
test_runner: do not crash on stdout that mimics a v8 frame
lib/internal/test_runner/runner.js65 + / 39 −
@@ -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); } }test/parallel/test-runner-v8-deserializer.mjs110 + / 0 −
@@ -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}...}`;