diff --git a/lib/fs.js b/lib/fs.js index e1406596c583..0f13d6245c0e 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -111,6 +111,8 @@ const { getValidatedFd, getValidatedPath, handleErrorFromBinding, + isDirectoryPath, + join: joinPath, preprocessSymlinkDestination, Stats, getReadFileBuffer, @@ -1749,8 +1751,8 @@ function mkdirSync(path, options) { * string results in `context.prefixes`). `result` is a `binding.readdir()` * result with file types, so only symbolic links and entries of unknown type * need a stat() to find out whether they lead to a directory. - * @param {string} dir - * @param {string} prefix + * @param {string | Buffer} dir + * @param {string | Buffer} prefix * @param {[string[], number[]]} result * @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context */ @@ -1758,23 +1760,24 @@ function collectRecursiveReaddirResult(dir, prefix, { 0: names, 1: types }, cont const { length } = names; for (let i = 0; i < length; i++) { const name = names[i]; - const relative = prefix === '' ? name : `${prefix}${pathModule.sep}${name}`; + const relative = prefix === '' ? name : joinPath(prefix, name); + const fullPath = joinPath(dir, name); let isDirectory; if (context.withFileTypes) { const dirent = getDirent(dir, name, types[i]); ArrayPrototypePush(context.results, dirent); // Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663 isDirectory = dirent.isDirectory() || - (dirent.isSymbolicLink() && binding.internalModuleStat(pathModule.join(dir, name)) === 1); + (dirent.isSymbolicLink() && isDirectoryPath(fullPath)); } else { ArrayPrototypePush(context.results, relative); const type = types[i]; isDirectory = type === UV_DIRENT_DIR || ((type === UV_DIRENT_LINK || type === UV_DIRENT_UNKNOWN) && - binding.internalModuleStat(pathModule.join(dir, name)) === 1); + isDirectoryPath(fullPath)); } if (isDirectory) { - ArrayPrototypePush(context.dirs, pathModule.join(dir, name)); + ArrayPrototypePush(context.dirs, fullPath); ArrayPrototypePush(context.prefixes, relative); } } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index a9d03ebb1d18..6d8ce3da98cf 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -74,6 +74,8 @@ const { getValidatedPath, getReadFileBuffer, getReadFileBufferByteLengthName, + isDirectoryPath, + join: joinPath, preprocessSymlinkDestination, stringToFlags, stringToSymlinkType, @@ -1662,7 +1664,8 @@ async function readdirRecursive(originalPath, options) { const { 0: path, 1: prefix, 2: { 0: names, 1: types } } = ArrayPrototypePop(queue); for (let i = 0; i < names.length; i++) { const name = names[i]; - const relative = prefix === '' ? name : `${prefix}${pathModule.sep}${name}`; + const relative = prefix === '' ? name : joinPath(prefix, name); + const direntPath = joinPath(path, name); let isDirectory; if (withFileTypes) { const dirent = getDirent(path, name, types[i]); @@ -1674,10 +1677,9 @@ async function readdirRecursive(originalPath, options) { const type = types[i]; isDirectory = type === UV_DIRENT_DIR || ((type === UV_DIRENT_LINK || type === UV_DIRENT_UNKNOWN) && - binding.internalModuleStat(pathModule.join(path, name)) === 1); + isDirectoryPath(direntPath)); } if (isDirectory) { - const direntPath = pathModule.join(path, name); ArrayPrototypePush(queue, [direntPath, relative, await readdirWithTypes(direntPath)]); } } diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 70aaa7e7c58e..b3db7678775b 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -65,6 +65,7 @@ const { validateUint32, } = require('internal/validators'); const pathModule = require('path'); +const binding = internalBinding('fs'); const kType = Symbol('type'); const kStats = Symbol('stats'); const kPartialAtimeNs = Symbol('partialAtimeNs'); @@ -249,6 +250,14 @@ function join(path, name) { 'path', ['string', 'Buffer'], path); } +function isDirectoryPath(path) { + if (typeof path === 'string') { + return binding.internalModuleStat(path) === 1; + } + const stats = binding.stat(path, false, undefined, false); + return stats !== undefined && getStatsFromBinding(stats).isDirectory(); +} + function getDirents(path, { 0: names, 1: types }, callback) { let i; if (typeof callback === 'function') { @@ -1128,6 +1137,8 @@ module.exports = { getDirent, getDirents, getOptions, + isDirectoryPath, + join, getValidatedFd, getValidatedPath, handleErrorFromBinding, diff --git a/test/parallel/test-fs-readdir-recursive-buffer.js b/test/parallel/test-fs-readdir-recursive-buffer.js new file mode 100644 index 000000000000..734bf8f12fc2 --- /dev/null +++ b/test/parallel/test-fs-readdir-recursive-buffer.js @@ -0,0 +1,49 @@ +'use strict'; + +// Regression test for https://github.com/nodejs/node/issues/58892 +// `readdir`/`readdirSync` with `{ recursive: true }` throw +// ERR_INVALID_ARG_TYPE when `encoding: 'buffer'` is used, because the +// internal recursive walk joins path segments with `path.join()`, which +// does not accept Buffer arguments. + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +const nested = path.join(tmpdir.path, 'a', 'b'); +fs.mkdirSync(nested, { recursive: true }); +fs.writeFileSync(path.join(nested, 'file.txt'), 'hello'); + +// readdirSync +const syncResult = fs.readdirSync(tmpdir.path, { recursive: true, encoding: 'buffer' }); +assert.ok(syncResult.every((entry) => Buffer.isBuffer(entry))); +assert.ok(syncResult.some((entry) => entry.toString().includes('file.txt'))); + +// readdirSync with withFileTypes +const syncDirents = fs.readdirSync( + tmpdir.path, + { recursive: true, encoding: 'buffer', withFileTypes: true } +); +assert.ok(syncDirents.some((dirent) => dirent.name.toString() === 'file.txt')); + +// readdir (callback) +fs.readdir( + tmpdir.path, + { recursive: true, encoding: 'buffer' }, + common.mustSucceed((entries) => { + assert.ok(entries.every((entry) => Buffer.isBuffer(entry))); + assert.ok(entries.some((entry) => entry.toString().includes('file.txt'))); + }) +); + +// fs.promises.readdir +fs.promises + .readdir(tmpdir.path, { recursive: true, encoding: 'buffer' }) + .then(common.mustCall((entries) => { + assert.ok(entries.every((entry) => Buffer.isBuffer(entry))); + assert.ok(entries.some((entry) => entry.toString().includes('file.txt'))); + }));