Skip to content

test_runner: parent runner still crashes on child stdout bytes that mimic an event frame (survives the #64706 fix) #66164

Description

@fan56

Current behavior

node --test crashes the parent runner (exit code 1; the failing component is the parent's stdout frame parser, not any test assertion) with:

Error: Unable to deserialize cloned data due to invalid or unsupported version.
    at #processRawBuffer (node:internal/test_runner/runner:497:20)
    at FileTest.parseMessage (node:internal/test_runner/runner:404:29)
    at Socket.<anonymous> (node:internal/test_runner/runner:552:15)

How often: deterministic — 10/10 runs on every version below. Control (same file minus the stdout write): 10/10 green on every version.

node has #64706 >>> 0 fix repro control
v24.21.0 yes (verified in shipped runner.js source) 10/10 crash 10/10 pass
v26.4.0 no (compiled-in source still has the signed read) 10/10 crash 10/10 pass
v22.23.2 no (v22 backport tracked in #65934) 10/10 crash 10/10 pass

Expected behavior

Anything a test (or code it imports) writes to stdout — text, banners, progress bars, QR codes, binary — should not be able to take down the parent runner. At worst it should degrade to test:stdout output or a per-file diagnostic, not abort the whole run with an error pointing at runner internals.

Steps to reproduce

Save as poison.test.mjs, then run node --test poison.test.mjs:

import { test } from 'node:test'

test('writes bytes that mimic a frame header', () => {
  const poison = Buffer.from([
    0xFF, 0x0F,             // frame magic (v8 serdes header tag)
    0x00, 0x00, 0x00, 0x08, // fake payload size = 8
    0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, // "ABCDEFGH" payload
  ])
  process.stdout.write(poison)
})

test('subsequent test whose real frames must also be parsed', () => {
  // trivial
})

Removing the process.stdout.write(poison) line makes it 10/10 green on all three versions.

Analysis

In lib/internal/test_runner/runner.js:

  • runTestFile spawns each test file with stdio: ['pipe', 'pipe', 'pipe'] and pipes the child's raw stdout into FileTest.parseMessage → #processRawBuffer.
  • The child's stdout therefore carries two unrelated producers on one pipe: the runner's own length-prefixed frames (FF 0F | uint32 BE size | v8 payload) and arbitrary application output. There is no isolation or escape mechanism for user bytes.
  • The first loop in #processRawBuffer skips bytes up to the next FF 0F magic and reports them as test:stdout. But after consuming a frame, the second loop continues without re-checking the magic — it trusts the next 4 bytes as a size and hands the slice straight to readHeader() / readValue(), with no try/catch anywhere on this path (the escaping stack above is the proof).
  • test_runner: convert to uint during deserialization #64706 hardened the size read itself (>>> 0), which fixes the negative/garbage-size case. What remains — and what the repro above hits on v24.21.0 — is the rest of the path: any stdout bytes that begin with FF 0F plus a plausible small size pass every guard, readHeader() throws on the payload, and the exception takes down the entire run.
  • Binary output containing FF 0F at a buffer head reaches exactly this state; plain text mostly takes the negative-size branch instead, which pre-test_runner: convert to uint during deserialization #64706 silently drops buffered frames (see test_runner: backport #64706 to v22.x — non-ASCII test stdout can silently drop a whole test file #65934) rather than crashing.

Real-world impact: one test's stdout can abort the whole node --test run with an internals-only error, which is hard for users to act on. We hit this as CI flakiness: a test printed a large terminal-QR block graphic and CI failed intermittently with the identical error; removing that output made the failures disappear.

Possible directions (not exhaustive): re-validate the FF 0F magic in the second loop before trusting the size; treat readHeader() / readValue() failure as lost/stray-output diagnostics for that file instead of a fatal exception; or an escaping mechanism so user stdout is never frame-parsed.

Related

Secondary repro (plain text, intermittent)

The deterministic file above is a minimized stand-in. The organic trigger is chunk-boundary dependent — text landing in the same parse buffer as the end of a frame gets its bytes read as a size. This file matches our real-world case (QR-style block text) and crashes only intermittently:

import { test } from 'node:test'

const tick = () => new Promise((r) => setTimeout(r, 0))

test('passes some ordinary assertions', async () => { await tick() })

test('prints a large block of text like a QR code / ASCII banner', async () => {
  for (let i = 0; i < 54; i++) {
    console.log('█▀▄ '.repeat(25))
    await tick() // let runner event frames flush between text writes
  }
})

test('keeps running tests after the noisy one', async () => { await tick() })

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    test_runnerIssues and PRs related to the test runner subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions