diff --git a/node/_fs/_fs_readdir.ts b/node/_fs/_fs_readdir.ts index e70eadd66e76..d2a3788020c3 100644 --- a/node/_fs/_fs_readdir.ts +++ b/node/_fs/_fs_readdir.ts @@ -1,6 +1,8 @@ import { asyncIterableToCallback } from "./_fs_watch.ts"; import Dirent from "./_fs_dirent.ts"; -import { fromFileUrl } from "../path.ts"; +import { denoErrorToNodeError } from "../_errors.ts"; +import { getValidatedPath } from "../internal/fs/utils.js"; +import { Buffer } from "../buffer.ts"; function toDirent(val: Deno.DirEntry): Dirent { return new Dirent(val); @@ -20,18 +22,18 @@ type readDirBoth = ( ) => void; export function readdir( - path: string | URL, + path: string | Buffer | URL, options: { withFileTypes?: false; encoding?: string }, callback: readDirCallback, ): void; export function readdir( - path: string | URL, + path: string | Buffer | URL, options: { withFileTypes: true; encoding?: string }, callback: readDirCallbackDirent, ): void; export function readdir(path: string | URL, callback: readDirCallback): void; export function readdir( - path: string | URL, + path: string | Buffer | URL, optionsOrCallback: readDirOptions | readDirCallback | readDirCallbackDirent, maybeCallback?: readDirCallback | readDirCallbackDirent, ) { @@ -43,7 +45,7 @@ export function readdir( ? optionsOrCallback : null; const result: Array = []; - path = path instanceof URL ? fromFileUrl(path) : path; + path = getValidatedPath(path); if (!callback) throw new Error("No callback function supplied"); @@ -58,7 +60,7 @@ export function readdir( } try { - asyncIterableToCallback(Deno.readDir(path), (val, done) => { + asyncIterableToCallback(Deno.readDir(path.toString()), (val, done) => { if (typeof path !== "string") return; if (done) { callback(null, result); @@ -67,9 +69,11 @@ export function readdir( if (options?.withFileTypes) { result.push(toDirent(val)); } else result.push(decode(val.name)); + }, (e) => { + callback(denoErrorToNodeError(e as Error, { syscall: "readdir" })); }); - } catch (error) { - callback(error instanceof Error ? error : new Error("[non-error thrown]")); + } catch (e) { + callback(denoErrorToNodeError(e as Error, { syscall: "readdir" })); } } @@ -83,19 +87,19 @@ function decode(str: string, encoding?: string): string { } export function readdirSync( - path: string | URL, + path: string | Buffer | URL, options: { withFileTypes: true; encoding?: string }, ): Dirent[]; export function readdirSync( - path: string | URL, + path: string | Buffer | URL, options?: { withFileTypes?: false; encoding?: string }, ): string[]; export function readdirSync( - path: string | URL, + path: string | Buffer | URL, options?: readDirOptions, ): Array { const result = []; - path = path instanceof URL ? fromFileUrl(path) : path; + path = getValidatedPath(path); if (options?.encoding) { try { @@ -107,10 +111,14 @@ export function readdirSync( } } - for (const file of Deno.readDirSync(path)) { - if (options?.withFileTypes) { - result.push(toDirent(file)); - } else result.push(decode(file.name)); + try { + for (const file of Deno.readDirSync(path.toString())) { + if (options?.withFileTypes) { + result.push(toDirent(file)); + } else result.push(decode(file.name)); + } + } catch (e) { + throw denoErrorToNodeError(e as Error, { syscall: "readdir" }); } return result; } diff --git a/node/_fs/_fs_watch.ts b/node/_fs/_fs_watch.ts index 25179392c403..11d7a1b0ff41 100644 --- a/node/_fs/_fs_watch.ts +++ b/node/_fs/_fs_watch.ts @@ -22,6 +22,7 @@ export function asyncIterableIteratorToCallback( export function asyncIterableToCallback( iter: AsyncIterable, callback: (val: T, done?: boolean) => void, + errCallback: (e: unknown) => void, ) { const iterator = iter[Symbol.asyncIterator](); function next() { @@ -32,7 +33,7 @@ export function asyncIterableToCallback( } callback(obj.value); next(); - }); + }, errCallback); } next(); } @@ -91,6 +92,8 @@ export function watch( asyncIterableToCallback(iterator, (val, done) => { if (done) return; fsWatcher.emit("change", val.kind, val.paths[0]); + }, (e) => { + fsWatcher.emit("error", e); }); return fsWatcher; diff --git a/node/_tools/config.json b/node/_tools/config.json index 354ec54b864d..4a4d661b1425 100644 --- a/node/_tools/config.json +++ b/node/_tools/config.json @@ -149,6 +149,8 @@ "test-fs-chmod-mask.js", "test-fs-chmod.js", "test-fs-chown-type-check.js", + "test-fs-readdir-stack-overflow.js", + "test-fs-readdir.js", "test-fs-readfile-empty.js", "test-fs-rm.js", "test-fs-rmdir-recursive-sync-warns-not-found.js", @@ -369,6 +371,7 @@ "parallel": [ "test-child-process-fork.js", "test-fs-access.js", + "test-fs-readdir-stack-overflow.js", "test-fs-write-file-invalid-path.js", "test-fs-write-file-sync.js", "test-fs-write-file.js", diff --git a/node/_tools/suites/parallel/test-fs-readdir-stack-overflow.js b/node/_tools/suites/parallel/test-fs-readdir-stack-overflow.js new file mode 100644 index 000000000000..bbc14903a546 --- /dev/null +++ b/node/_tools/suites/parallel/test-fs-readdir-stack-overflow.js @@ -0,0 +1,26 @@ +// deno-fmt-ignore-file +// deno-lint-ignore-file + +// Copyright Joyent and Node contributors. All rights reserved. MIT license. +// Taken from Node 16.13.0 +// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually + +'use strict'; + +require('../common'); + +const assert = require('assert'); +const fs = require('fs'); + +function recurse() { + fs.readdirSync('.'); + recurse(); +} + +assert.throws( + () => recurse(), + { + name: 'RangeError', + message: 'Maximum call stack size exceeded' + } +); diff --git a/node/_tools/suites/parallel/test-fs-readdir.js b/node/_tools/suites/parallel/test-fs-readdir.js new file mode 100644 index 000000000000..4f7bdbeec2ff --- /dev/null +++ b/node/_tools/suites/parallel/test-fs-readdir.js @@ -0,0 +1,60 @@ +// deno-fmt-ignore-file +// deno-lint-ignore-file + +// Copyright Joyent and Node contributors. All rights reserved. MIT license. +// Taken from Node 16.13.0 +// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually + +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); + +const tmpdir = require('../common/tmpdir'); + +const readdirDir = tmpdir.path; +const files = ['empty', 'files', 'for', 'just', 'testing']; + +// Make sure tmp directory is clean +tmpdir.refresh(); + +// Create the necessary files +files.forEach(function(currentFile) { + fs.closeSync(fs.openSync(`${readdirDir}/${currentFile}`, 'w')); +}); + +// Check the readdir Sync version +assert.deepStrictEqual(files, fs.readdirSync(readdirDir).sort()); + +// Check the readdir async version +fs.readdir(readdirDir, common.mustSucceed((f) => { + assert.deepStrictEqual(files, f.sort()); +})); + +// readdir() on file should throw ENOTDIR +// https://github.com/joyent/node/issues/1869 +assert.throws(function() { + fs.readdirSync(__filename); +}, /Error: ENOTDIR: not a directory/); + +fs.readdir(__filename, common.mustCall(function(e) { + assert.strictEqual(e.code, 'ENOTDIR'); +})); + +[false, 1, [], {}, null, undefined].forEach((i) => { + assert.throws( + () => fs.readdir(i, common.mustNotCall()), + { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + } + ); + assert.throws( + () => fs.readdirSync(i), + { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + } + ); +}); diff --git a/node/internal_binding/_libuv_winerror.ts b/node/internal_binding/_libuv_winerror.ts index a9d93812df57..9cc255dc6fec 100644 --- a/node/internal_binding/_libuv_winerror.ts +++ b/node/internal_binding/_libuv_winerror.ts @@ -153,7 +153,7 @@ export function uvTranslateSysError(sysErrno: number): string { case winErrors.ERROR_BAD_PATHNAME: return "ENOENT"; case winErrors.ERROR_DIRECTORY: - return "ENOENT"; + return "ENOTDIR"; case winErrors.ERROR_ENVVAR_NOT_FOUND: return "ENOENT"; case winErrors.ERROR_FILE_NOT_FOUND: