From afc629428554a76162ebef275d6c8e4b4611bc4d Mon Sep 17 00:00:00 2001 From: Ethan Arrowood Date: Wed, 6 Nov 2024 12:11:22 +0000 Subject: [PATCH] fs: fix bufferSize option for opendir recursive The bufferSize option was not respected in recursive mode. This PR implements a naive solution to fix this issue as, until a better implementation can be designed. Fixes: https://github.com/nodejs/node/issues/48820 --- lib/internal/fs/dir.js | 8 ++++---- test/sequential/test-fs-opendir-recursive.js | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js index 0293e23c44bc33..49361503d8e027 100644 --- a/lib/internal/fs/dir.js +++ b/lib/internal/fs/dir.js @@ -164,12 +164,12 @@ class Dir { return; } - const result = handle.read( + // Fully read the directory and buffer the entries. + let result; + while ((result = handle.read( this.#options.encoding, this.#options.bufferSize, - ); - - if (result) { + )) !== undefined) { this.processReadResult(path, result); } diff --git a/test/sequential/test-fs-opendir-recursive.js b/test/sequential/test-fs-opendir-recursive.js index a7e9b9a318e5fb..494e5591491b2f 100644 --- a/test/sequential/test-fs-opendir-recursive.js +++ b/test/sequential/test-fs-opendir-recursive.js @@ -132,6 +132,7 @@ function getDirentPath(dirent) { } function assertDirents(dirents) { + assert.strictEqual(dirents.length, expected.length); dirents.sort((a, b) => (getDirentPath(a) < getDirentPath(b) ? -1 : 1)); assert.deepStrictEqual( dirents.map((dirent) => { @@ -221,3 +222,21 @@ function processDirCb(dir, cb) { test().then(common.mustCall()); } + +// Issue https://github.com/nodejs/node/issues/48820 highlights that +// opendir recursive does not properly handle the buffer size option. +// This test asserts that the buffer size option is respected. +{ + const dir = fs.opendirSync(testDir, { bufferSize: 1, recursive: true }); + processDirSync(dir); + dir.closeSync(); +} + +{ + fs.opendir(testDir, { recursive: true, bufferSize: 1 }, common.mustSucceed((dir) => { + processDirCb(dir, common.mustSucceed((dirents) => { + assertDirents(dirents); + dir.close(common.mustSucceed()); + })); + })); +}