test_runner: parent runner still crashes on child stdout bytes that mimic an event frame (survives the #64706 fix)
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 4/5
- Thời gian dự kiến
- 3-5 ngày
- Mức phù hợp với người mới
- 68/100
- Loại issue
- Lỗi
- Độ rõ ràng
- Đặc tả rõ ràng
- Mức độ hoạt động
- Sôi nổi
- Công nghệ
- javascript
- Lĩnh vực
- testing-qa
Hướng nghiên cứu
Start in lib/internal/test_runner/runner.js, tracing FileTest.parseMessage into #processRawBuffer, and run the deterministic poison.test.mjs reproduction with node --test. The fix is done when arbitrary child stdout, including bytes resembling a frame, no longer crashes the parent and subsequent test frames are still processed; verify the control and reproduction cases.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
- Version: v24.21.0 (this release includes the
>>> 0fix from #64706); also verified on v26.4.0 and v22.23.2 - Platform: macOS 26.6.2 (25G83), arm64
- Subsystem: test_runner
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:
runTestFilespawns each test file withstdio: ['pipe', 'pipe', 'pipe']and pipes the child's raw stdout intoFileTest.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
#processRawBufferskips bytes up to the nextFF 0Fmagic and reports them astest: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 toreadHeader()/readValue(), with no try/catch anywhere on this path (the escaping stack above is the proof). - #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 withFF 0Fplus a plausible small size pass every guard,readHeader()throws on the payload, and the exception takes down the entire run. - Binary output containing
FF 0Fat a buffer head reaches exactly this state; plain text mostly takes the negative-size branch instead, which pre-#64706 silently drops buffered frames (see #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
- #64061 — signed size-read crash (same error/stack); fixed by #64706. This issue is about what that fix does not cover.
- #65934 — pending v22.x backport of #64706.
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() })
- Ngôn ngữ chính
- JavaScript
- Star
- 122k
- Fork
- 37.4k
- Merge trung bình
- 4 ngày 4 giờ
- Pull request đã merge (30 ngày)
- 276
Hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của nodejs/node
-
doc
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 65/100
-
build
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 88/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 84/100
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 90/100
-
feature request
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 68/100
Issue tương tự
-
ai-observability bug team/ai-observability
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 68/100
AllTheMods/ATM-10-L#19 ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
SignalK/n2k-signalk#345 ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 84/100
-
Độ khó 1/5 1-3 giờ Mức phù hợp với người mới 88/100
JuliaPluto/PlutoPlotly.jl#72 ·