diff --git a/benchmark/fs/bench-readdir-recursive.js b/benchmark/fs/bench-readdir-recursive.js new file mode 100644 index 000000000000..a17416fd1137 --- /dev/null +++ b/benchmark/fs/bench-readdir-recursive.js @@ -0,0 +1,47 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const path = require('path'); +const assert = require('assert'); + +const bench = common.createBenchmark(main, { + n: [10], + dir: ['lib', 'test/parallel'], + mode: ['sync', 'callback', 'promise'], + withFileTypes: ['true', 'false'], +}); + +async function main({ n, dir, mode, withFileTypes }) { + withFileTypes = withFileTypes === 'true'; + const fullPath = path.resolve(__dirname, '../../', dir); + const options = { recursive: true, withFileTypes }; + let entries; + + bench.start(); + switch (mode) { + case 'sync': + for (let i = 0; i < n; i++) { + entries = fs.readdirSync(fullPath, options); + } + break; + case 'callback': + for (let i = 0; i < n; i++) { + entries = await new Promise((resolve, reject) => { + fs.readdir(fullPath, options, (err, result) => { + if (err) reject(err); + else resolve(result); + }); + }); + } + break; + case 'promise': + for (let i = 0; i < n; i++) { + entries = await fs.promises.readdir(fullPath, options); + } + break; + } + bench.end(n); + + assert.ok(entries.length > 0); +} diff --git a/lib/fs.js b/lib/fs.js index d1823ce97298..1e642d42bfd6 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -28,7 +28,6 @@ const { ArrayFromAsync, ArrayPrototypePush, BigIntPrototypeToString, - Boolean, FunctionPrototypeCall, MathMax, Number, @@ -57,9 +56,6 @@ const { F_OK, O_WRONLY, O_SYMLINK, - UV_DIRENT_DIR, - UV_DIRENT_LINK, - UV_DIRENT_UNKNOWN, } = constants; const pathModule = require('path'); @@ -105,8 +101,8 @@ const { }, copyObject, Dirent, - getDirent, getDirents, + getRecursiveDirents, getOptions, getValidatedFd, getValidatedPath, @@ -1742,43 +1738,6 @@ function mkdirSync(path, options) { } } -/** - * Appends one directory's entries to `context.results` and the subdirectories - * still to visit to `context.dirs` (with the prefix their entries get in - * 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[], number[]]} result - * @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context - */ -function collectRecursiveReaddirResult(dir, prefix, { 0: names, 1: types }, context) { - const { length } = names; - for (let i = 0; i < length; i++) { - const name = names[i]; - const relative = prefix === '' ? name : `${prefix}${pathModule.sep}${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); - } 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); - } - if (isDirectory) { - ArrayPrototypePush(context.dirs, pathModule.join(dir, name)); - ArrayPrototypePush(context.prefixes, relative); - } - } -} - /* * 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 @@ -1792,79 +1751,30 @@ function collectRecursiveReaddirResult(dir, prefix, { 0: names, 1: types }, cont * @returns {void} */ function readdirRecursive(basePath, options, callback) { - const context = { - withFileTypes: Boolean(options.withFileTypes), - results: [], - dirs: [basePath], - prefixes: [''], + const withFileTypes = !!options.withFileTypes; + const req = new FSReqCallback(); + req.oncomplete = (err, result) => { + if (err) { + callback(err); + return; + } + callback(null, withFileTypes ? getRecursiveDirents(basePath, result) : result); }; - - let i = 0; - - /** - * Reads one directory from `context.dirs` and then moves on to the next - * one, or calls back once none are left. - * @param {string} path - * @param {string} prefix path of this directory relative to `basePath` - */ - function read(path, prefix) { - const req = new FSReqCallback(); - req.oncomplete = (err, result) => { - if (err) { - callback(err); - return; - } - - if (result === undefined) { - callback(null, context.results); - return; - } - - try { - collectRecursiveReaddirResult(path, prefix, result, context); - } catch (err) { - callback(err); - return; - } - - if (i < context.dirs.length) { - read(context.dirs[i], context.prefixes[i++]); - } else { - callback(null, context.results); - } - }; - - binding.readdir(path, options.encoding, true, req); - } - - read(context.dirs[i], context.prefixes[i++]); + binding.readdirRecursive(basePath, options.encoding, withFileTypes, req); } /** - * 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 - * `binding.readdir`. - * @param {string} basePath + * Synchronously reads the entire contents of the `basePath` directory. + * This function does not validate `basePath` as a directory. It is passed + * directly to `binding.readdirRecursive`. + * @param {string | Buffer} basePath * @param {{ encoding: string, withFileTypes: boolean }} options - * @returns {string[] | Dirent[]} + * @returns {string[] | Buffer[] | Dirent[]} */ function readdirSyncRecursive(basePath, options) { - const context = { - withFileTypes: Boolean(options.withFileTypes), - results: [], - dirs: [basePath], - prefixes: [''], - }; - - for (let i = 0; i < context.dirs.length; i++) { - const dir = context.dirs[i]; - const result = binding.readdir(dir, options.encoding, true); - if (result !== undefined) { - collectRecursiveReaddirResult(dir, context.prefixes[i], result, context); - } - } - - return context.results; + const withFileTypes = !!options.withFileTypes; + const result = binding.readdirRecursive(basePath, options.encoding, withFileTypes); + return result !== undefined && withFileTypes ? getRecursiveDirents(basePath, result) : result; } /** @@ -1898,9 +1808,6 @@ function readdir(path, options, callback) { } if (options.recursive) { - // Make shallow copy to prevent mutating options from affecting results - options = copyObject(options); - readdirRecursive(path, options, callback); return; } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index ce69512fa5c9..d3ef4820c558 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -1,7 +1,6 @@ 'use strict'; const { - ArrayPrototypePop, ArrayPrototypePush, Error, ErrorCaptureStackTrace, @@ -32,9 +31,6 @@ const { O_WRONLY, S_IFMT, S_IFREG, - UV_DIRENT_DIR, - UV_DIRENT_LINK, - UV_DIRENT_UNKNOWN, } = constants; const binding = internalBinding('fs'); @@ -66,8 +62,8 @@ const { kWriteFileMaxChunkSize, }, copyObject, - getDirent, getDirents, + getRecursiveDirents, getOptions, getStatFsFromBinding, getStatsFromBinding, @@ -1643,41 +1639,17 @@ async function mkdir(path, options) { async function readdirRecursive(originalPath, options) { const withFileTypes = !!options.withFileTypes; - const readdirWithTypes = (path) => PromisePrototypeThen( - binding.readdir(path, options.encoding, true, kUsePromises), + const result = await PromisePrototypeThen( + binding.readdirRecursive( + originalPath, + options.encoding, + withFileTypes, + kUsePromises, + ), undefined, handleErrorFromBinding, ); - const result = []; - const queue = [[originalPath, '', await readdirWithTypes(originalPath)]]; - - while (queue.length > 0) { - // If we want to implement BFS make this a `shift` call instead of `pop` - 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}`; - let isDirectory; - if (withFileTypes) { - const dirent = getDirent(path, name, types[i]); - ArrayPrototypePush(result, dirent); - isDirectory = dirent.isDirectory(); - } else { - ArrayPrototypePush(result, relative); - // Entries that are, or may be, symbolic links to directories are followed. - const type = types[i]; - isDirectory = type === UV_DIRENT_DIR || - ((type === UV_DIRENT_LINK || type === UV_DIRENT_UNKNOWN) && - binding.internalModuleStat(pathModule.join(path, name)) === 1); - } - if (isDirectory) { - const direntPath = pathModule.join(path, name); - ArrayPrototypePush(queue, [direntPath, relative, await readdirWithTypes(direntPath)]); - } - } - } - - return result; + return withFileTypes ? getRecursiveDirents(originalPath, result) : result; } async function readdir(path, options) { diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 70aaa7e7c58e..e03cb745c148 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -2,6 +2,7 @@ const { ArrayIsArray, + ArrayPrototypePush, BigInt, Date, DateNow, @@ -323,6 +324,26 @@ function getDirent(path, name, type, callback) { } } +/** + * Builds the Dirent objects for a recursive readdir + * @param {string | Buffer} basePath + * @param {[string[] | Buffer[], number[], number[], string[]]} result + * @returns {Dirent[]} + */ +function getRecursiveDirents(basePath, { 0: names, 1: types, 2: dirIndices, 3: dirs }) { + const parentPaths = [basePath]; + if (dirs.length > 1) { + const base = typeof basePath === 'string' ? basePath : `${basePath}`; + for (let i = 1; i < dirs.length; i++) { + ArrayPrototypePush(parentPaths, pathModule.join(base, dirs[i])); + } + } + for (let i = 0; i < names.length; i++) { + names[i] = new Dirent(names[i], types[i], parentPaths[dirIndices[i]]); + } + return names; +} + function getOptions(options, defaultOptions = kEmptyObject) { if (options == null || typeof options === 'function') { return defaultOptions; @@ -1128,6 +1149,7 @@ module.exports = { getDirent, getDirents, getOptions, + getRecursiveDirents, getValidatedFd, getValidatedPath, handleErrorFromBinding, diff --git a/src/node_file.cc b/src/node_file.cc index b2e765346cf1..a0e78585468a 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2252,6 +2252,358 @@ static void ReadDir(const FunctionCallbackInfo& args) { } } +namespace { + +// Maps an st_mode to the uv_dirent_type_t that uv_fs_scandir would report. +uv_dirent_type_t DirentTypeFromMode(uint64_t mode) { + switch (mode & S_IFMT) { + case S_IFREG: + return UV_DIRENT_FILE; + case S_IFDIR: + return UV_DIRENT_DIR; + case S_IFLNK: + return UV_DIRENT_LINK; + case S_IFCHR: + return UV_DIRENT_CHAR; +#ifdef S_IFIFO + case S_IFIFO: + return UV_DIRENT_FIFO; +#endif +#ifdef S_IFSOCK + case S_IFSOCK: + return UV_DIRENT_SOCKET; +#endif +#ifdef S_IFBLK + case S_IFBLK: + return UV_DIRENT_BLOCK; +#endif + default: + return UV_DIRENT_UNKNOWN; + } +} + +std::string JoinDirEntry(const std::string& dir, const char* name) { + std::string result = dir; +#ifdef _WIN32 + const bool has_separator = + !result.empty() && (result.back() == '\\' || result.back() == '/'); +#else + const bool has_separator = !result.empty() && result.back() == '/'; +#endif + if (!has_separator) result += kPathSeparator; + result += name; + return result; +} + +struct RecursiveReadDirEntry { + // Path relative to the base directory, e.g. "sub/dir/name". + std::string path; + // Offset of the entry name within `path`. + uint32_t name_offset; + // Index into RecursiveReadDirResult::dirs of the containing directory. + uint32_t dir_index; + uv_dirent_type_t type; +}; + +struct RecursiveReadDirResult { + std::vector entries; + // Relative paths of every scanned directory, in scan order. + std::vector dirs; + // A uv error code, kAccessDenied (1), or 0 on success. + int error = 0; + std::string error_path; + + static constexpr int kAccessDenied = 1; +}; + +// Reads the whole tree below `base` breadth-first. Directories scanned in +// the order they were discovered. Symbolic links to directories are followed, +// matching fs.readdir(). +void ReadDirRecursiveWalk(Environment* env, + const std::string& base, + RecursiveReadDirResult* out) { + std::vector full_paths{base}; + out->dirs.emplace_back(); + + const bool check_permissions = env->permission()->enabled(); + + for (size_t i = 0; i < full_paths.size(); i++) { + // Copies: both vectors grow while this directory is being scanned. + const std::string dir = full_paths[i]; + const std::string rel = out->dirs[i]; + + if (check_permissions && i > 0 && + !env->permission()->is_granted( + env, permission::PermissionScope::kFileSystemRead, dir) && + !env->permission()->warning_only()) { + out->error = RecursiveReadDirResult::kAccessDenied; + out->error_path = dir; + return; + } + + uv_fs_t req; + int r = uv_fs_scandir(nullptr, &req, dir.c_str(), 0, nullptr); + if (r < 0) { + uv_fs_req_cleanup(&req); + out->error = r; + out->error_path = dir; + return; + } + + for (;;) { + uv_dirent_t ent; + r = uv_fs_scandir_next(&req, &ent); + if (r == UV_EOF) break; + if (r < 0) { + uv_fs_req_cleanup(&req); + out->error = r; + out->error_path = dir; + return; + } + + RecursiveReadDirEntry entry; + entry.path = rel; + if (!entry.path.empty()) entry.path += kPathSeparator; + entry.name_offset = static_cast(entry.path.size()); + entry.path += ent.name; + entry.dir_index = static_cast(i); + entry.type = ent.type; + + bool is_dir = ent.type == UV_DIRENT_DIR; + if (ent.type == UV_DIRENT_UNKNOWN || ent.type == UV_DIRENT_LINK) { + // The file system did not report a type, or the entry is a symlink + // that may point at a directory + std::string full = JoinDirEntry(dir, ent.name); + uv_fs_t stat_req; + if (ent.type == UV_DIRENT_UNKNOWN) { + if (uv_fs_lstat(nullptr, &stat_req, full.c_str(), nullptr) == 0) { + entry.type = DirentTypeFromMode(stat_req.statbuf.st_mode); + } + uv_fs_req_cleanup(&stat_req); + is_dir = entry.type == UV_DIRENT_DIR; + } + if (entry.type == UV_DIRENT_LINK) { + if (uv_fs_stat(nullptr, &stat_req, full.c_str(), nullptr) == 0) { + is_dir = S_ISDIR(stat_req.statbuf.st_mode); + } + uv_fs_req_cleanup(&stat_req); + } + } + + if (is_dir) { + full_paths.push_back(JoinDirEntry(dir, ent.name)); + out->dirs.push_back(entry.path); + } + out->entries.push_back(std::move(entry)); + } + + uv_fs_req_cleanup(&req); + } +} + +// Without file types the result is an array of relative paths. With file +// types it is [names, types, dirIndices, dirs]. +// See `getRecursiveDirents` for how the array is used later on. +MaybeLocal MarshalRecursiveReadDir(Isolate* isolate, + const RecursiveReadDirResult& result, + enum encoding encoding, + bool with_types) { + EscapableHandleScope scope(isolate); + const size_t count = result.entries.size(); + + LocalVector names(isolate); + names.reserve(count); + + if (!with_types) { + for (const RecursiveReadDirEntry& entry : result.entries) { + Local name; + if (!StringBytes::Encode( + isolate, entry.path.data(), entry.path.size(), encoding) + .ToLocal(&name)) { + return MaybeLocal(); + } + names.push_back(name); + } + return scope.Escape(Array::New(isolate, names.data(), names.size())); + } + + LocalVector types(isolate); + LocalVector dir_indices(isolate); + types.reserve(count); + dir_indices.reserve(count); + for (const RecursiveReadDirEntry& entry : result.entries) { + Local name; + if (!StringBytes::Encode(isolate, + entry.path.data() + entry.name_offset, + entry.path.size() - entry.name_offset, + encoding) + .ToLocal(&name)) { + return MaybeLocal(); + } + names.push_back(name); + types.push_back(Integer::New(isolate, entry.type)); + dir_indices.push_back(Integer::NewFromUnsigned(isolate, entry.dir_index)); + } + + LocalVector dirs(isolate); + dirs.reserve(result.dirs.size()); + for (const std::string& dir : result.dirs) { + Local path; + if (!StringBytes::Encode(isolate, dir.data(), dir.size(), UTF8) + .ToLocal(&path)) { + return MaybeLocal(); + } + dirs.push_back(path); + } + + Local parts[] = { + Array::New(isolate, names.data(), names.size()), + Array::New(isolate, types.data(), types.size()), + Array::New(isolate, dir_indices.data(), dir_indices.size()), + Array::New(isolate, dirs.data(), dirs.size()), + }; + return scope.Escape(Array::New(isolate, parts, arraysize(parts))); +} + +// Runs a recursive readdir on the libuv thread pool and settles the request +// on the main thread. +class ReadDirRecursiveWork final : public ThreadPoolWork { + public: + ReadDirRecursiveWork(Environment* env, + FSReqBase* req_wrap, + std::string path, + enum encoding encoding, + bool with_types) + : ThreadPoolWork(env, "readdir_recursive"), + req_wrap_(req_wrap), + path_(std::move(path)), + encoding_(encoding), + with_types_(with_types) {} + + void Walk() { ReadDirRecursiveWalk(env(), path_, &result_); } + + void DoThreadPoolWork() override { Walk(); } + + void AfterThreadPoolWork(int status) override { Finish(status); } + + // Settles the request and deletes this. + void Finish(int status) { + std::unique_ptr self(this); + Environment* env = this->env(); + Isolate* isolate = env->isolate(); + HandleScope handle_scope(isolate); + Context::Scope context_scope(env->context()); + + // Release me even if the environment is shutting down + BaseObjectPtr req_wrap = std::move(req_wrap_); + req_wrap->Detach(); + + FS_ASYNC_TRACE_END1(UV_FS_SCANDIR, req_wrap.get(), "result", result_.error) + if (!env->can_call_into_js()) return; + + if (status < 0) { + return req_wrap->Reject( + UVException(isolate, status, "scandir", nullptr, path_.c_str())); + } + if (result_.error == RecursiveReadDirResult::kAccessDenied) { + return permission::Permission::AsyncThrowAccessDenied( + env, + req_wrap.get(), + permission::PermissionScope::kFileSystemRead, + result_.error_path); + } + if (result_.error < 0) { + return req_wrap->Reject(UVException(isolate, + result_.error, + "scandir", + nullptr, + result_.error_path.c_str())); + } + + Local value; + TryCatch try_catch(isolate); + if (!MarshalRecursiveReadDir(isolate, result_, encoding_, with_types_) + .ToLocal(&value)) { + CHECK(try_catch.CanContinue()); + return req_wrap->Reject(try_catch.Exception()); + } + req_wrap->Resolve(value); + } + + private: + BaseObjectPtr req_wrap_; + std::string path_; + enum encoding encoding_; + bool with_types_; + RecursiveReadDirResult result_; +}; + +} // namespace + +// readdirRecursive(path, encoding, withTypes[, req]) +static void ReadDirRecursive(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + + const int argc = args.Length(); + CHECK_GE(argc, 3); + + BufferValue path(isolate, args[0]); + CHECK_NOT_NULL(*path); + ToNamespacedPath(env, &path); + + const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); + + bool with_types = args[2]->IsTrue(); + + if (argc > 3) { // readdirRecursive(path, encoding, withTypes, req) + FSReqBase* req_wrap_async = GetReqWrap(args, 3); + CHECK_NOT_NULL(req_wrap_async); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemRead, + path.ToStringView()); + req_wrap_async->Init("scandir", nullptr, 0, encoding); + FS_ASYNC_TRACE_BEGIN1( + UV_FS_SCANDIR, req_wrap_async, "path", TRACE_STR_COPY(*path)) + auto work = std::make_unique( + env, req_wrap_async, path.ToString(), encoding, with_types); + if (env->permission()->enabled()) { + // Permission checks are only valid on the main thread + work->Walk(); + env->SetImmediate([work = std::move(work)](Environment* env) mutable { + work.release()->Finish(0); + }); + } else { + work.release()->ScheduleWork(); + } + } else { // readdirRecursive(path, encoding, withTypes) + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemRead, path.ToStringView()); + env->PrintSyncTrace(); + FS_SYNC_TRACE_BEGIN(readdir); + RecursiveReadDirResult result; + ReadDirRecursiveWalk(env, path.ToString(), &result); + FS_SYNC_TRACE_END(readdir); + + if (result.error == RecursiveReadDirResult::kAccessDenied) { + return permission::Permission::ThrowAccessDenied( + env, permission::PermissionScope::kFileSystemRead, result.error_path); + } + if (result.error < 0) { + return env->ThrowUVException( + result.error, "scandir", nullptr, result.error_path.c_str()); + } + + Local value; + if (MarshalRecursiveReadDir(isolate, result, encoding, with_types) + .ToLocal(&value)) { + args.GetReturnValue().Set(value); + } + } +} + static inline Maybe AsyncCheckOpenPermissions(Environment* env, FSReqBase* req_wrap, const BufferValue& path, @@ -4560,6 +4912,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, SetMethod(isolate, target, "rmSync", RmSync); SetMethod(isolate, target, "mkdir", MKDir); SetMethod(isolate, target, "readdir", ReadDir); + SetMethod(isolate, target, "readdirRecursive", ReadDirRecursive); SetMethod(isolate, target, "internalModuleStat", InternalModuleStat); SetMethod(isolate, target, "stat", Stat); SetMethod(isolate, target, "lstat", LStat); @@ -4698,6 +5051,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(RmSync); registry->Register(MKDir); registry->Register(ReadDir); + registry->Register(ReadDirRecursive); registry->Register(InternalModuleStat); registry->Register(Stat); registry->Register(LStat); diff --git a/test/known_issues/test-fs-readdir-promise-recursive-with-buffer.js b/test/known_issues/test-fs-readdir-promise-recursive-with-buffer.js deleted file mode 100644 index 314d6dda9a2d..000000000000 --- a/test/known_issues/test-fs-readdir-promise-recursive-with-buffer.js +++ /dev/null @@ -1,15 +0,0 @@ -'use strict'; - -// This test will fail because the implementation does not properly -// handle the case when the path is a Buffer and the function is called -// in recursive mode. - -// Refs: https://github.com/nodejs/node/issues/58892 - -const common = require('../common'); - -const { readdir } = require('node:fs/promises'); -const { join } = require('node:path'); - -const testDirPath = join(__dirname, '..', '..'); -readdir(Buffer.from(testDirPath), { recursive: true }).then(common.mustCall()); diff --git a/test/known_issues/test-fs-readdir-recursive-with-buffer.js b/test/known_issues/test-fs-readdir-recursive-with-buffer.js deleted file mode 100644 index c2eae8f428fe..000000000000 --- a/test/known_issues/test-fs-readdir-recursive-with-buffer.js +++ /dev/null @@ -1,15 +0,0 @@ -'use strict'; - -// This test will fail because the implementation does not properly -// handle the case when the path is a Buffer and the function is called -// in recursive mode. - -// Refs: https://github.com/nodejs/node/issues/58892 - -const common = require('../common'); - -const { readdir } = require('node:fs'); -const { join } = require('node:path'); - -const testDirPath = join(__dirname, '..', '..'); -readdir(Buffer.from(testDirPath), { recursive: true }, common.mustSucceed()); diff --git a/test/known_issues/test-fs-readdir-sync-recursive-with-buffer.js b/test/known_issues/test-fs-readdir-sync-recursive-with-buffer.js deleted file mode 100644 index 0f60c290f15d..000000000000 --- a/test/known_issues/test-fs-readdir-sync-recursive-with-buffer.js +++ /dev/null @@ -1,15 +0,0 @@ -'use strict'; - -// This test will fail because the implementation does not properly -// handle the case when the path is a Buffer and the function is called -// in recursive mode. - -// Refs: https://github.com/nodejs/node/issues/58892 - -require('../common'); - -const { readdirSync } = require('node:fs'); -const { join } = require('node:path'); - -const testDirPath = join(__dirname, '..', '..'); -readdirSync(Buffer.from(testDirPath), { recursive: true }); diff --git a/test/parallel/test-fs-readdir-recursive-tree.js b/test/parallel/test-fs-readdir-recursive-tree.js new file mode 100644 index 000000000000..b4e25c483a7d --- /dev/null +++ b/test/parallel/test-fs-readdir-recursive-tree.js @@ -0,0 +1,208 @@ +'use strict'; + +// Exercises the native recursive readdir (sync, callback and promise forms) +// against a reference walk built from the non-recursive API. + +const common = require('../common'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +const root = tmpdir.resolve('tree'); +fs.mkdirSync(path.join(root, 'a', 'b', 'c'), { recursive: true }); +fs.mkdirSync(path.join(root, 'd')); +fs.mkdirSync(path.join(root, '.hidden')); +fs.mkdirSync(path.join(root, 'empty')); +fs.writeFileSync(path.join(root, 'top'), ''); +fs.writeFileSync(path.join(root, 'a', '1'), ''); +fs.writeFileSync(path.join(root, 'a', 'b', '2'), ''); +fs.writeFileSync(path.join(root, 'a', 'b', 'c', '3'), ''); +fs.writeFileSync(path.join(root, 'd', '4'), ''); +fs.writeFileSync(path.join(root, '.hidden', '5'), ''); + +const canSymlink = common.canCreateSymLink(); +if (canSymlink) { + fs.symlinkSync(path.join(root, 'a'), path.join(root, 'd', 'link-to-a'), 'dir'); + fs.symlinkSync(path.join(root, 'a', '1'), path.join(root, 'd', 'link-to-file')); + fs.symlinkSync(path.join(root, 'nowhere'), path.join(root, 'd', 'dangling')); +} + +// Reference implementation: breadth-first, entries in readdir order, +// directory symlinks followed (fs.readdir semantics). +function reference(basePath) { + const entries = []; + const queue = [basePath]; + for (let i = 0; i < queue.length; i++) { + const dir = queue[i]; + for (const dirent of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, dirent.name); + entries.push({ + relative: path.relative(basePath, full), + isDirent: true, + name: dirent.name, + parentPath: dir, + isDirectory: dirent.isDirectory(), + isSymbolicLink: dirent.isSymbolicLink(), + isFile: dirent.isFile(), + }); + let isDir = dirent.isDirectory(); + if (!isDir && dirent.isSymbolicLink()) { + try { + isDir = fs.statSync(full).isDirectory(); + } catch { + isDir = false; + } + } + if (isDir) queue.push(full); + } + } + return entries; +} + +function fromDirents(dirents) { + return dirents.map((dirent) => { + return { + isDirent: dirent instanceof fs.Dirent, + name: dirent.name, + parentPath: dirent.parentPath, + isDirectory: dirent.isDirectory(), + isSymbolicLink: dirent.isSymbolicLink(), + isFile: dirent.isFile(), + }; + }); +} + +function stripRelative(entries) { + return entries.map(({ relative, ...rest }) => rest); +} + +const expected = reference(root); +const expectedPaths = expected.map((entry) => entry.relative); +const expectedDirents = stripRelative(expected); + +// The tree is only interesting if symlinks were followed. +if (canSymlink) { + assert(expectedPaths.includes(path.join('d', 'link-to-a', 'b', 'c', '3'))); + assert(!expectedPaths.includes(path.join('d', 'link-to-file', '1'))); +} +assert(expectedPaths.includes(path.join('.hidden', '5'))); +assert(expectedPaths.includes('empty')); + +// Sync. +assert.deepStrictEqual(fs.readdirSync(root, { recursive: true }), expectedPaths); +assert.deepStrictEqual( + fromDirents(fs.readdirSync(root, { recursive: true, withFileTypes: true })), + expectedDirents, +); + +// Callback. +fs.readdir(root, { recursive: true }, common.mustSucceed((paths) => { + assert.deepStrictEqual(paths, expectedPaths); +})); +fs.readdir(root, { recursive: true, withFileTypes: true }, common.mustSucceed((dirents) => { + assert.deepStrictEqual(fromDirents(dirents), expectedDirents); +})); + +// Promises. +(async () => { + assert.deepStrictEqual( + await fs.promises.readdir(root, { recursive: true }), + expectedPaths, + ); + assert.deepStrictEqual( + fromDirents(await fs.promises.readdir(root, { recursive: true, withFileTypes: true })), + expectedDirents, + ); +})().then(common.mustCall()); + +// Mutating the options object after the call must not affect the result. +{ + const options = { recursive: true, withFileTypes: true }; + fs.readdir(root, options, common.mustSucceed((dirents) => { + assert.deepStrictEqual(fromDirents(dirents), expectedDirents); + })); + options.withFileTypes = false; + options.recursive = false; +} + +// Relative paths are relative to the path as given, while parentPath is the +// path as given for the top level and a joined path below it. +{ + const relativeRoot = path.relative(process.cwd(), root) + path.sep; + const paths = fs.readdirSync(relativeRoot, { recursive: true }); + assert.deepStrictEqual(paths, expectedPaths); + const dirents = fs.readdirSync(relativeRoot, { recursive: true, withFileTypes: true }); + assert.strictEqual(dirents[0].parentPath, relativeRoot); + const nested = dirents.find((dirent) => dirent.name === '1'); + assert.strictEqual(nested.parentPath, path.join(relativeRoot, 'a')); +} + +// Buffer encodings and Buffer paths. +{ + const buffers = fs.readdirSync(root, { recursive: true, encoding: 'buffer' }); + assert(buffers.every((entry) => Buffer.isBuffer(entry))); + assert.deepStrictEqual(buffers.map(String), expectedPaths); + + const dirents = fs.readdirSync(root, { recursive: true, encoding: 'buffer', withFileTypes: true }); + assert(dirents.every((dirent) => Buffer.isBuffer(dirent.name))); + assert.deepStrictEqual( + dirents.map((dirent) => ({ ...fromDirents([dirent])[0], name: String(dirent.name) })), + expectedDirents, + ); + + assert.deepStrictEqual(fs.readdirSync(Buffer.from(root), { recursive: true }), expectedPaths); + fs.readdir(Buffer.from(root), { recursive: true }, common.mustSucceed((paths) => { + assert.deepStrictEqual(paths, expectedPaths); + })); +} + +// Errors carry the directory that failed, for the root and below it. +{ + const missing = path.join(root, 'missing'); + assert.throws(() => fs.readdirSync(missing, { recursive: true }), { + code: 'ENOENT', + syscall: 'scandir', + path: missing, + }); + const file = path.join(root, 'top'); + assert.throws(() => fs.readdirSync(file, { recursive: true }), { + code: 'ENOTDIR', + syscall: 'scandir', + path: file, + }); + fs.readdir(missing, { recursive: true }, common.mustCall((err) => { + assert.strictEqual(err.code, 'ENOENT'); + assert.strictEqual(err.syscall, 'scandir'); + assert.strictEqual(err.path, missing); + })); + assert.rejects(fs.promises.readdir(file, { recursive: true }), { + code: 'ENOTDIR', + syscall: 'scandir', + path: file, + }).then(common.mustCall()); +} + +if (!common.isWindows && !common.isIBMi && process.getuid() !== 0) { + const locked = tmpdir.resolve('locked'); + const inner = path.join(locked, 'inner'); + fs.mkdirSync(inner, { recursive: true }); + fs.chmodSync(inner, 0); + try { + assert.throws(() => fs.readdirSync(locked, { recursive: true }), { + code: 'EACCES', + syscall: 'scandir', + path: inner, + }); + fs.readdir(locked, { recursive: true }, common.mustCall((err) => { + assert.strictEqual(err.code, 'EACCES'); + assert.strictEqual(err.path, inner); + fs.chmodSync(inner, 0o755); + })); + } catch (err) { + fs.chmodSync(inner, 0o755); + throw err; + } +}