nodejs/node · #66273

test_runner: do not crash on stdout that mimics a v8 frame

faizanu94 · merged Oct 1, 20262 files · 175 + / 39 −
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}...}`;