Skip to content

Commit

Permalink
fs: fix readdir and opendir recursive with unknown file types
Browse files Browse the repository at this point in the history
If the libuv operations invoked by `readdir`/`opendir` return
`uv_dirent_t` values where the `type` is `UV_DIRENT_UNKNOWN` then a
further `lstat` is issued to fully construct the `Dirent` values. In the
recursive versions of these functions, the `path` parameter was
incorrectly assumed to be the path to the entry when it should be the
path to the directory containing the entry.

Fixes nodejs#49499.

PR-URL: nodejs#49603
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
  • Loading branch information
lux01 authored and alexfernandez committed Nov 1, 2023
1 parent 7c69730 commit f89f85d
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 19 deletions.
9 changes: 5 additions & 4 deletions lib/internal/fs/dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class Dir {
ArrayPrototypePush(
this[kDirBufferedEntries],
getDirent(
pathModule.join(path, result[i]),
path,
result[i],
result[i + 1],
),
Expand All @@ -161,9 +161,10 @@ class Dir {
}

readSyncRecursive(dirent) {
const ctx = { path: dirent.path };
const path = pathModule.join(dirent.path, dirent.name);
const ctx = { path };
const handle = dirBinding.opendir(
pathModule.toNamespacedPath(dirent.path),
pathModule.toNamespacedPath(path),
this[kDirOptions].encoding,
undefined,
ctx,
Expand All @@ -177,7 +178,7 @@ class Dir {
);

if (result) {
this.processReadResult(dirent.path, result);
this.processReadResult(path, result);
}

handle.close(undefined, ctx);
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ function join(path, name) {
}

if (typeof path === 'string' && typeof name === 'string') {
return pathModule.basename(path) === name ? path : pathModule.join(path, name);
return pathModule.join(path, name);
}

if (isUint8Array(path) && isUint8Array(name)) {
Expand Down
17 changes: 10 additions & 7 deletions test/sequential/test-fs-opendir-recursive.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const tmpdir = require('../common/tmpdir');
const testDir = tmpdir.path;

const fileStructure = [
[ 'a', [ 'foo', 'bar' ] ],
[ 'a', [ 'a', 'foo', 'bar' ] ],
[ 'b', [ 'foo', 'bar' ] ],
[ 'c', [ 'foo', 'bar' ] ],
[ 'd', [ 'foo', 'bar' ] ],
Expand Down Expand Up @@ -91,7 +91,7 @@ fs.symlinkSync(symlinkTargetFile, pathModule.join(symlinksRootPath, 'symlink-src
fs.symlinkSync(symlinkTargetDir, pathModule.join(symlinksRootPath, 'symlink-src-dir'));

const expected = [
'a', 'a/bar', 'a/foo', 'aa', 'aa/bar', 'aa/foo',
'a', 'a/a', 'a/bar', 'a/foo', 'aa', 'aa/bar', 'aa/foo',
'abc', 'abc/def', 'abc/def/bar', 'abc/def/foo', 'abc/ghi', 'abc/ghi/bar', 'abc/ghi/foo',
'b', 'b/bar', 'b/foo', 'bb', 'bb/bar', 'bb/foo',
'c', 'c/bar', 'c/foo', 'cc', 'cc/bar', 'cc/foo',
Expand Down Expand Up @@ -128,15 +128,18 @@ for (let i = 0; i < expected.length; i++) {
}

function getDirentPath(dirent) {
return pathModule.relative(testDir, dirent.path);
return pathModule.relative(testDir, pathModule.join(dirent.path, dirent.name));
}

function assertDirents(dirents) {
dirents.sort((a, b) => (getDirentPath(a) < getDirentPath(b) ? -1 : 1));
for (const [i, dirent] of dirents.entries()) {
assert(dirent instanceof fs.Dirent);
assert.strictEqual(getDirentPath(dirent), expected[i]);
}
assert.deepStrictEqual(
dirents.map((dirent) => {
assert(dirent instanceof fs.Dirent);
return getDirentPath(dirent);
}),
expected
);
}

function processDirSync(dir) {
Expand Down
17 changes: 10 additions & 7 deletions test/sequential/test-fs-readdir-recursive.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const tmpdir = require('../common/tmpdir');
const readdirDir = tmpdir.path;

const fileStructure = [
[ 'a', [ 'foo', 'bar' ] ],
[ 'a', [ 'a', 'foo', 'bar' ] ],
[ 'b', [ 'foo', 'bar' ] ],
[ 'c', [ 'foo', 'bar' ] ],
[ 'd', [ 'foo', 'bar' ] ],
Expand Down Expand Up @@ -90,7 +90,7 @@ fs.symlinkSync(symlinkTargetFile, pathModule.join(symlinksRootPath, 'symlink-src
fs.symlinkSync(symlinkTargetDir, pathModule.join(symlinksRootPath, 'symlink-src-dir'));

const expected = [
'a', 'a/bar', 'a/foo', 'aa', 'aa/bar', 'aa/foo',
'a', 'a/a', 'a/bar', 'a/foo', 'aa', 'aa/bar', 'aa/foo',
'abc', 'abc/def', 'abc/def/bar', 'abc/def/foo', 'abc/ghi', 'abc/ghi/bar', 'abc/ghi/foo',
'b', 'b/bar', 'b/foo', 'bb', 'bb/bar', 'bb/foo',
'c', 'c/bar', 'c/foo', 'cc', 'cc/bar', 'cc/foo',
Expand Down Expand Up @@ -133,11 +133,14 @@ function getDirentPath(dirent) {
function assertDirents(dirents) {
assert.strictEqual(dirents.length, expected.length);
dirents.sort((a, b) => (getDirentPath(a) < getDirentPath(b) ? -1 : 1));
for (const [i, dirent] of dirents.entries()) {
assert(dirent instanceof fs.Dirent);
assert.notStrictEqual(dirent.name, undefined);
assert.strictEqual(getDirentPath(dirent), expected[i]);
}
assert.deepStrictEqual(
dirents.map((dirent) => {
assert(dirent instanceof fs.Dirent);
assert.notStrictEqual(dirent.name, undefined);
return getDirentPath(dirent);
}),
expected
);
}

// readdirSync
Expand Down

0 comments on commit f89f85d

Please sign in to comment.