From ed9a05cfc28fe9f10c1f5d7751c0257ed23f7490 Mon Sep 17 00:00:00 2001 From: Christian Aurich Zanettini Martins Date: Thu, 22 Oct 2026 18:38:28 -0300 Subject: [PATCH] fs: fix readFileSync utf8 overflow on short reads `ReadFileUtf8()` appends reads shorter than 8 KiB to a string and only moves to a heap buffer at the first full 8 KiB read. That buffer always started at 64 KiB, but a pipe can deliver more than that in short reads first, so copying the string into it overflowed the heap. With `big_len` past `big_cap`, the next read size also underflowed. Size the heap buffer to at least what was already read. When that is more than 64 KiB, the buffer is full and the existing growth step enlarges it before the next read. Signed-off-by: Christian Aurich Zanettini Martins --- src/node_file.cc | 5 +- .../test-fs-readfilesync-pipe-short-reads.js | 59 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-fs-readfilesync-pipe-short-reads.js diff --git a/src/node_file.cc b/src/node_file.cc index f5e7cc80e724..85790fbda9ab 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3552,9 +3552,10 @@ static void ReadFileUtf8(const FunctionCallbackInfo& args) { if (static_cast(r) < sizeof(buffer)) { continue; } - // Switch to the heap buffer. + // Switch to the heap buffer. A pipe can deliver more than kMinChunk in + // short reads before the first full one. uv_fs_req_cleanup(&req); - big_cap = kMinChunk; + big_cap = std::max(kMinChunk, result.size()); big = UncheckedMalloc(big_cap); if (big == nullptr) { FS_SYNC_TRACE_END(read); diff --git a/test/parallel/test-fs-readfilesync-pipe-short-reads.js b/test/parallel/test-fs-readfilesync-pipe-short-reads.js new file mode 100644 index 000000000000..0d4fa6c98b14 --- /dev/null +++ b/test/parallel/test-fs-readfilesync-pipe-short-reads.js @@ -0,0 +1,59 @@ +'use strict'; +// fs.readFileSync(fd, 'utf8') collects short reads in a growing string and +// only moves to a heap buffer at the first full 8 KiB read. On a pipe, more +// than 64 KiB (the heap buffer's first size) can arrive in short reads before +// that happens. +// Refs: https://github.com/nodejs/node/issues/66341 +const common = require('../common'); + +if (!common.isLinux) + common.skip('Paces the writes with /proc//io.'); + +const assert = require('assert'); +const { spawn } = require('child_process'); +const fs = require('fs'); + +const kShortWrite = 4000; +const kShortWrites = 20; +const kTail = 100_000; +const expected = 'x'.repeat(kShortWrite * kShortWrites) + 'y'.repeat(kTail); + +if (process.argv[2] === 'child') { + fs.writeSync(1, 'ready\n'); + assert.strictEqual(fs.readFileSync(0, 'utf8'), expected); + return; +} + +// Bytes the process has received from read() and similar calls. +function readBytes(pid) { + const io = fs.readFileSync(`/proc/${pid}/io`, 'latin1'); + return Number(/^rchar: (\d+)$/m.exec(io)[1]); +} + +const child = spawn(process.execPath, [__filename, 'child'], { + stdio: ['pipe', 'pipe', 'inherit'], +}); + +child.stdout.once('data', common.mustCall(() => { + // Each short write waits until the child, blocked in readFileSync(), has + // read everything sent before it, so no two of them can be merged into a + // full read. + const start = readBytes(child.pid); + let sent = 0; + function writeNext() { + if (readBytes(child.pid) - start < sent) { + setTimeout(writeNext, 1); + } else if (sent < kShortWrite * kShortWrites) { + child.stdin.write('x'.repeat(kShortWrite)); + sent += kShortWrite; + setTimeout(writeNext, 1); + } else { + child.stdin.end('y'.repeat(kTail)); + } + } + writeNext(); +})); + +child.on('exit', common.mustCall((code, signal) => { + assert.deepStrictEqual({ code, signal }, { code: 0, signal: null }); +}));