Skip to content

Commit 373b9a5

Browse files
committed
lib: fix readdir in recursive mode with a buffer argument
calling readdir with a buffer arguemnt and the recursive flag produces an ERR_INVALID_ARG_TYPE error. This is to fix that. Fixes: #58892
1 parent 6432765 commit 373b9a5

File tree

5 files changed

+62
-21
lines changed

5 files changed

+62
-21
lines changed

lib/fs.js

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1394,6 +1394,31 @@ function readdirRecursive(basePath, options, callback) {
13941394
read(context.pathsQueue[i++]);
13951395
}
13961396

1397+
function joinPath(base, name) {
1398+
if (BufferIsBuffer(base)) {
1399+
const baseStr = BufferToString(base);
1400+
const nameStr = BufferIsBuffer(name) ? BufferToString(name) : name;
1401+
return Buffer.from(pathModule.join(baseStr, nameStr));
1402+
}
1403+
return pathModule.join(base, name);
1404+
}
1405+
1406+
function relativePath(from, to) {
1407+
if (BufferIsBuffer(from)) {
1408+
const fromStr = BufferToString(from);
1409+
const toStr = BufferIsBuffer(to) ? BufferToString(to) : to;
1410+
return Buffer.from(pathModule.relative(fromStr, toStr));
1411+
}
1412+
return pathModule.relative(from, to);
1413+
}
1414+
1415+
function toPathString(path) {
1416+
if (BufferIsBuffer(path)) {
1417+
return BufferToString(path);
1418+
}
1419+
return path;
1420+
}
1421+
13971422
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
13981423
// The first array is the names, and the second array is the types.
13991424
// They are guaranteed to be the same length; hence, setting `length` to the length
@@ -1407,21 +1432,21 @@ function handleDirents({ result, currentPath, context }) {
14071432
for (let i = 0; i < length; i++) {
14081433
// Avoid excluding symlinks, as they are not directories.
14091434
// Refs: https://github.com/nodejs/node/issues/52663
1410-
const fullPath = pathModule.join(currentPath, names[i]);
1435+
const fullPath = joinPath(currentPath, names[i]);
14111436
const dirent = getDirent(currentPath, names[i], types[i]);
14121437
ArrayPrototypePush(context.readdirResults, dirent);
14131438

1414-
if (dirent.isDirectory() || binding.internalModuleStat(fullPath) === 1) {
1439+
if (dirent.isDirectory() || binding.internalModuleStat(toPathString(fullPath)) === 1) {
14151440
ArrayPrototypePush(context.pathsQueue, fullPath);
14161441
}
14171442
}
14181443
}
14191444

14201445
function handleFilePaths({ result, currentPath, context }) {
14211446
for (let i = 0; i < result.length; i++) {
1422-
const resultPath = pathModule.join(currentPath, result[i]);
1423-
const relativeResultPath = pathModule.relative(context.basePath, resultPath);
1424-
const stat = binding.internalModuleStat(resultPath);
1447+
const resultPath = joinPath(currentPath, result[i]);
1448+
const relativeResultPath = relativePath(context.basePath, resultPath);
1449+
const stat = binding.internalModuleStat(toPathString(resultPath));
14251450
ArrayPrototypePush(context.readdirResults, relativeResultPath);
14261451

14271452
if (stat === 1) {

lib/internal/fs/promises.js

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,31 @@ async function mkdir(path, options) {
870870
);
871871
}
872872

873+
function joinPath(base, name) {
874+
if (BufferIsBuffer(base)) {
875+
const baseStr = BufferToString(base);
876+
const nameStr = BufferIsBuffer(name) ? BufferToString(name) : name;
877+
return Buffer.from(pathModule.join(baseStr, nameStr));
878+
}
879+
return pathModule.join(base, name);
880+
}
881+
882+
function relativePath(from, to) {
883+
if (BufferIsBuffer(from)) {
884+
const fromStr = BufferToString(from);
885+
const toStr = BufferIsBuffer(to) ? BufferToString(to) : to;
886+
return Buffer.from(pathModule.relative(fromStr, toStr));
887+
}
888+
return pathModule.relative(from, to);
889+
}
890+
891+
function toPathString(path) {
892+
if (BufferIsBuffer(path)) {
893+
return BufferToString(path);
894+
}
895+
return path;
896+
}
897+
873898
async function readdirRecursive(originalPath, options) {
874899
const result = [];
875900
const queue = [
@@ -896,7 +921,7 @@ async function readdirRecursive(originalPath, options) {
896921
for (const dirent of getDirents(path, readdir)) {
897922
ArrayPrototypePush(result, dirent);
898923
if (dirent.isDirectory()) {
899-
const direntPath = pathModule.join(path, dirent.name);
924+
const direntPath = joinPath(path, dirent.name);
900925
ArrayPrototypePush(queue, [
901926
direntPath,
902927
await PromisePrototypeThen(
@@ -917,11 +942,11 @@ async function readdirRecursive(originalPath, options) {
917942
while (queue.length > 0) {
918943
const { 0: path, 1: readdir } = ArrayPrototypePop(queue);
919944
for (const ent of readdir) {
920-
const direntPath = pathModule.join(path, ent);
921-
const stat = binding.internalModuleStat(direntPath);
945+
const direntPath = joinPath(path, ent);
946+
const stat = binding.internalModuleStat(toPathString(direntPath));
922947
ArrayPrototypePush(
923948
result,
924-
pathModule.relative(originalPath, direntPath),
949+
relativePath(originalPath, direntPath),
925950
);
926951
if (stat === 1) {
927952
ArrayPrototypePush(queue, [

test/known_issues/test-fs-readdir-promise-recursive-with-buffer.js renamed to test/parallel/test-fs-readdir-promise-recursive-with-buffer.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
'use strict';
22

3-
// This test will fail because the the implementation does not properly
4-
// handle the case when the path is a Buffer and the function is called
5-
// in recursive mode.
6-
3+
// Test that fs.promises.readdir works with Buffer paths in recursive mode.
74
// Refs: https://github.com/nodejs/node/issues/58892
85

96
const common = require('../common');

test/known_issues/test-fs-readdir-recursive-with-buffer.js renamed to test/parallel/test-fs-readdir-recursive-with-buffer.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
'use strict';
22

3-
// This test will fail because the the implementation does not properly
4-
// handle the case when the path is a Buffer and the function is called
5-
// in recursive mode.
6-
3+
// Test that readdir callback works with Buffer paths in recursive mode.
74
// Refs: https://github.com/nodejs/node/issues/58892
85

96
const common = require('../common');

test/known_issues/test-fs-readdir-sync-recursive-with-buffer.js renamed to test/parallel/test-fs-readdir-sync-recursive-with-buffer.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
'use strict';
22

3-
// This test will fail because the the implementation does not properly
4-
// handle the case when the path is a Buffer and the function is called
5-
// in recursive mode.
6-
3+
// Test that readdirSync works with Buffer paths in recursive mode.
74
// Refs: https://github.com/nodejs/node/issues/58892
85

96
require('../common');

0 commit comments

Comments
 (0)