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

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];
     }
   }
 }

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}...}`;