fs: fix readdir and opendir recursive with unknown file types

lux01 · ruyadorno · commit db3fc6d0874d · 2023-09-27T22:32:46.000-04:00
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 #49499. PR-URL: #49603 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js
@@ -152,7 +152,7 @@ class Dir {
       ArrayPrototypePush(
         this[kDirBufferedEntries],
         getDirent(
-          pathModule.join(path, result[i]),
+          path,
           result[i],
           result[i + 1],
         ),
@@ -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,
@@ -177,7 +178,7 @@ class Dir {
     );
 
     if (result) {
-      this.processReadResult(dirent.path, result);
+      this.processReadResult(path, result);
     }
 
     handle.close(undefined, ctx);
diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
@@ -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)) {
diff --git a/test/sequential/test-fs-opendir-recursive.js b/test/sequential/test-fs-opendir-recursive.js
@@ -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' ] ],
@@ -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',
@@ -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) {
diff --git a/test/sequential/test-fs-readdir-recursive.js b/test/sequential/test-fs-readdir-recursive.js
@@ -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' ] ],
@@ -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',
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -234,7 +234,7 @@ function join(path, name) {`
`234`	`234`	`}`
`235`	`235`
`236`	`236`	`if (typeof path === 'string' && typeof name === 'string') {`
`237`		`- return pathModule.basename(path) === name ? path : pathModule.join(path, name);`
	`237`	`+ return pathModule.join(path, name);`
`238`	`238`	`}`
`239`	`239`
`240`	`240`	`if (isUint8Array(path) && isUint8Array(name)) {`