From 1d6cbc1ba55fd4f8f29ce9ce8be6cd16c14fa943 Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Thu, 5 Dec 2024 17:25:25 -0300 Subject: [PATCH] lib: fix `fs.readdir` recursive async MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/nodejs/node/issues/56006 PR-URL: https://github.com/nodejs/node/pull/56041 Reviewed-By: Ethan Arrowood Reviewed-By: Juan José Arboleda --- lib/fs.js | 153 +++++++++++++++++++++------- test/fixtures/permission/fs-read.js | 5 + 2 files changed, 119 insertions(+), 39 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 45b664b21ec84e..ae22dabfc79731 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1372,6 +1372,102 @@ function mkdirSync(path, options) { } } +/* + * An recursive algorithm for reading the entire contents of the `basePath` directory. + * This function does not validate `basePath` as a directory. It is passed directly to + * `binding.readdir`. + * @param {string} basePath + * @param {{ encoding: string, withFileTypes: boolean }} options + * @param {( + * err?: Error, + * files?: string[] | Buffer[] | Dirent[] + * ) => any} callback + * @returns {void} +*/ +function readdirRecursive(basePath, options, callback) { + const context = { + withFileTypes: Boolean(options.withFileTypes), + encoding: options.encoding, + basePath, + readdirResults: [], + pathsQueue: [basePath], + }; + + let i = 0; + + function read(path) { + const req = new FSReqCallback(); + req.oncomplete = (err, result) => { + if (err) { + callback(err); + return; + } + + if (result === undefined) { + callback(null, context.readdirResults); + return; + } + + processReaddirResult({ + result, + currentPath: path, + context, + }); + + if (i < context.pathsQueue.length) { + read(context.pathsQueue[i++]); + } else { + callback(null, context.readdirResults); + } + }; + + binding.readdir( + path, + context.encoding, + context.withFileTypes, + req, + ); + } + + read(context.pathsQueue[i++]); +} + +// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays. +// The first array is the names, and the second array is the types. +// They are guaranteed to be the same length; hence, setting `length` to the length +// of the first array within the result. +const processReaddirResult = (args) => (args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args)); + +function handleDirents({ result, currentPath, context }) { + const { 0: names, 1: types } = result; + const { length } = names; + + for (let i = 0; i < length; i++) { + // Avoid excluding symlinks, as they are not directories. + // Refs: https://github.com/nodejs/node/issues/52663 + const fullPath = pathModule.join(currentPath, names[i]); + const dirent = getDirent(currentPath, names[i], types[i]); + ArrayPrototypePush(context.readdirResults, dirent); + + if (dirent.isDirectory() || binding.internalModuleStat(binding, fullPath) === 1) { + ArrayPrototypePush(context.pathsQueue, fullPath); + } + } +} + +function handleFilePaths({ result, currentPath, context }) { + for (let i = 0; i < result.length; i++) { + const resultPath = pathModule.join(currentPath, result[i]); + const relativeResultPath = pathModule.relative(context.basePath, resultPath); + const stat = binding.internalModuleStat(binding, resultPath); + ArrayPrototypePush(context.readdirResults, relativeResultPath); + + if (stat === 1) { + ArrayPrototypePush(context.pathsQueue, resultPath); + } + } +} + /** * An iterative algorithm for reading the entire contents of the `basePath` directory. * This function does not validate `basePath` as a directory. It is passed directly to @@ -1381,58 +1477,37 @@ function mkdirSync(path, options) { * @returns {string[] | Dirent[]} */ function readdirSyncRecursive(basePath, options) { - const withFileTypes = Boolean(options.withFileTypes); - const encoding = options.encoding; - - const readdirResults = []; - const pathsQueue = [basePath]; + const context = { + withFileTypes: Boolean(options.withFileTypes), + encoding: options.encoding, + basePath, + readdirResults: [], + pathsQueue: [basePath], + }; function read(path) { const readdirResult = binding.readdir( path, - encoding, - withFileTypes, + context.encoding, + context.withFileTypes, ); if (readdirResult === undefined) { return; } - if (withFileTypes) { - // Calling `readdir` with `withFileTypes=true`, the result is an array of arrays. - // The first array is the names, and the second array is the types. - // They are guaranteed to be the same length; hence, setting `length` to the length - // of the first array within the result. - const length = readdirResult[0].length; - for (let i = 0; i < length; i++) { - // Avoid excluding symlinks, as they are not directories. - // Refs: https://github.com/nodejs/node/issues/52663 - const stat = binding.internalModuleStat(binding, pathModule.join(path, readdirResult[0][i])); - const dirent = getDirent(path, readdirResult[0][i], readdirResult[1][i]); - ArrayPrototypePush(readdirResults, dirent); - if (dirent.isDirectory() || stat === 1) { - ArrayPrototypePush(pathsQueue, pathModule.join(dirent.parentPath, dirent.name)); - } - } - } else { - for (let i = 0; i < readdirResult.length; i++) { - const resultPath = pathModule.join(path, readdirResult[i]); - const relativeResultPath = pathModule.relative(basePath, resultPath); - const stat = binding.internalModuleStat(binding, resultPath); - ArrayPrototypePush(readdirResults, relativeResultPath); - // 1 indicates directory - if (stat === 1) { - ArrayPrototypePush(pathsQueue, resultPath); - } - } - } + processReaddirResult({ + result: readdirResult, + currentPath: path, + context, + }); } - for (let i = 0; i < pathsQueue.length; i++) { - read(pathsQueue[i]); + for (let i = 0; i < context.pathsQueue.length; i++) { + read(context.pathsQueue[i]); } - return readdirResults; + return context.readdirResults; } /** @@ -1458,7 +1533,7 @@ function readdir(path, options, callback) { } if (options.recursive) { - callback(null, readdirSyncRecursive(path, options)); + readdirRecursive(path, options, callback); return; } diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js index 186117a6b768dd..fa4ea1207f50bd 100644 --- a/test/fixtures/permission/fs-read.js +++ b/test/fixtures/permission/fs-read.js @@ -289,6 +289,11 @@ const regularFile = __filename; permission: 'FileSystemRead', resource: path.toNamespacedPath(blockedFolder), })); + fs.readdir(blockedFolder, { recursive: true }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFolder), + })); assert.throws(() => { fs.readdirSync(blockedFolder); }, common.expectsError({