Skip to content

Commit 982a50e

Browse files
committed
lib: fix fs.readdir recursive async
Fixes: #56006
1 parent b17a1fb commit 982a50e

File tree

2 files changed

+126
-39
lines changed

2 files changed

+126
-39
lines changed

lib/fs.js

Lines changed: 121 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,6 +1369,109 @@ function mkdirSync(path, options) {
13691369
}
13701370
}
13711371

1372+
/*
1373+
* An recurssive algorithm for reading the entire contents of the `basePath` directory.
1374+
* This function does not validate `basePath` as a directory. It is passed directly to
1375+
* `binding.readdir`.
1376+
* @param {string} basePath
1377+
* @param {{ encoding: string, withFileTypes: boolean }} options
1378+
* @param {(
1379+
* err?: Error,
1380+
* files?: string[] | Buffer[] | Dirent[]
1381+
* ) => any} callback
1382+
* @returns {void}
1383+
*/
1384+
function readdirRecursive(basePath, options, callback) {
1385+
const context = {
1386+
withFileTypes: Boolean(options.withFileTypes),
1387+
encoding: options.encoding,
1388+
basePath,
1389+
readdirResults: [],
1390+
pathsQueue: [basePath],
1391+
};
1392+
1393+
let i = 0;
1394+
1395+
function read(path) {
1396+
const req = new FSReqCallback();
1397+
req.oncomplete = (err, result) => {
1398+
if (err) {
1399+
callback(err);
1400+
return;
1401+
}
1402+
1403+
if (result === undefined) {
1404+
callback(null, context.readdirResults);
1405+
return;
1406+
}
1407+
1408+
processReaddirResult({
1409+
result,
1410+
currentPath: path,
1411+
context,
1412+
});
1413+
1414+
if (i < context.pathsQueue.length) {
1415+
read(context.pathsQueue[i++]);
1416+
} else {
1417+
callback(null, context.readdirResults);
1418+
}
1419+
};
1420+
1421+
binding.readdir(
1422+
path,
1423+
context.encoding,
1424+
context.withFileTypes,
1425+
req,
1426+
);
1427+
}
1428+
1429+
read(context.pathsQueue[i++]);
1430+
}
1431+
1432+
function processReaddirResult({ result, currentPath, context }) {
1433+
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1434+
// The first array is the names, and the second array is the types.
1435+
// They are guaranteed to be the same length; hence, setting `length` to the length
1436+
// of the first array within the result.
1437+
if (context.withFileTypes) {
1438+
handleDirents({ result, currentPath, context });
1439+
} else {
1440+
handleFilePaths({ result, currentPath, context });
1441+
}
1442+
}
1443+
1444+
function handleDirents({ result, currentPath, context }) {
1445+
const { 0: names, 1: types } = result;
1446+
const length = names.length;
1447+
1448+
for (let i = 0; i < length; i++) {
1449+
// Avoid excluding symlinks, as they are not directories.
1450+
// Refs: https://github.com/nodejs/node/issues/52663
1451+
const fullPath = pathModule.join(currentPath, names[i]);
1452+
const stat = binding.internalModuleStat(binding, fullPath);
1453+
const dirent = getDirent(currentPath, names[i], types[i]);
1454+
ArrayPrototypePush(context.readdirResults, dirent);
1455+
1456+
if (dirent.isDirectory() || stat === 1) {
1457+
ArrayPrototypePush(context.pathsQueue, fullPath);
1458+
}
1459+
}
1460+
}
1461+
1462+
function handleFilePaths({ result, currentPath, context }) {
1463+
for (let i = 0; i < result.length; i++) {
1464+
const resultPath = pathModule.join(currentPath, result[i]);
1465+
const relativeResultPath = pathModule.relative(context.basePath, resultPath);
1466+
const stat = binding.internalModuleStat(binding, resultPath);
1467+
ArrayPrototypePush(context.readdirResults, relativeResultPath);
1468+
1469+
if (stat === 1) {
1470+
ArrayPrototypePush(context.pathsQueue, resultPath);
1471+
}
1472+
}
1473+
}
1474+
13721475
/**
13731476
* An iterative algorithm for reading the entire contents of the `basePath` directory.
13741477
* This function does not validate `basePath` as a directory. It is passed directly to
@@ -1378,58 +1481,37 @@ function mkdirSync(path, options) {
13781481
* @returns {string[] | Dirent[]}
13791482
*/
13801483
function readdirSyncRecursive(basePath, options) {
1381-
const withFileTypes = Boolean(options.withFileTypes);
1382-
const encoding = options.encoding;
1383-
1384-
const readdirResults = [];
1385-
const pathsQueue = [basePath];
1484+
const context = {
1485+
withFileTypes: Boolean(options.withFileTypes),
1486+
encoding: options.encoding,
1487+
basePath,
1488+
readdirResults: [],
1489+
pathsQueue: [basePath],
1490+
};
13861491

13871492
function read(path) {
13881493
const readdirResult = binding.readdir(
13891494
path,
1390-
encoding,
1391-
withFileTypes,
1495+
context.encoding,
1496+
context.withFileTypes,
13921497
);
13931498

13941499
if (readdirResult === undefined) {
13951500
return;
13961501
}
13971502

1398-
if (withFileTypes) {
1399-
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1400-
// The first array is the names, and the second array is the types.
1401-
// They are guaranteed to be the same length; hence, setting `length` to the length
1402-
// of the first array within the result.
1403-
const length = readdirResult[0].length;
1404-
for (let i = 0; i < length; i++) {
1405-
// Avoid excluding symlinks, as they are not directories.
1406-
// Refs: https://github.com/nodejs/node/issues/52663
1407-
const stat = binding.internalModuleStat(binding, pathModule.join(path, readdirResult[0][i]));
1408-
const dirent = getDirent(path, readdirResult[0][i], readdirResult[1][i]);
1409-
ArrayPrototypePush(readdirResults, dirent);
1410-
if (dirent.isDirectory() || stat === 1) {
1411-
ArrayPrototypePush(pathsQueue, pathModule.join(dirent.parentPath, dirent.name));
1412-
}
1413-
}
1414-
} else {
1415-
for (let i = 0; i < readdirResult.length; i++) {
1416-
const resultPath = pathModule.join(path, readdirResult[i]);
1417-
const relativeResultPath = pathModule.relative(basePath, resultPath);
1418-
const stat = binding.internalModuleStat(binding, resultPath);
1419-
ArrayPrototypePush(readdirResults, relativeResultPath);
1420-
// 1 indicates directory
1421-
if (stat === 1) {
1422-
ArrayPrototypePush(pathsQueue, resultPath);
1423-
}
1424-
}
1425-
}
1503+
processReaddirResult({
1504+
result: readdirResult,
1505+
currentPath: path,
1506+
context,
1507+
});
14261508
}
14271509

1428-
for (let i = 0; i < pathsQueue.length; i++) {
1429-
read(pathsQueue[i]);
1510+
for (let i = 0; i < context.pathsQueue.length; i++) {
1511+
read(context.pathsQueue[i]);
14301512
}
14311513

1432-
return readdirResults;
1514+
return context.readdirResults;
14331515
}
14341516

14351517
/**
@@ -1455,7 +1537,7 @@ function readdir(path, options, callback) {
14551537
}
14561538

14571539
if (options.recursive) {
1458-
callback(null, readdirSyncRecursive(path, options));
1540+
readdirRecursive(path, options, callback);
14591541
return;
14601542
}
14611543

test/fixtures/permission/fs-read.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,11 @@ const regularFile = __filename;
289289
permission: 'FileSystemRead',
290290
resource: path.toNamespacedPath(blockedFolder),
291291
}));
292+
fs.readdir(blockedFolder, { recursive: true }, common.expectsError({
293+
code: 'ERR_ACCESS_DENIED',
294+
permission: 'FileSystemRead',
295+
resource: path.toNamespacedPath(blockedFolder),
296+
}));
292297
assert.throws(() => {
293298
fs.readdirSync(blockedFolder);
294299
}, common.expectsError({

0 commit comments

Comments
 (0)