From f89f85de34d6cc99b31d71adfc60c7f7189175da Mon Sep 17 00:00:00 2001
From: William Marlow <william.marlow@ibm.com>
Date: Mon, 11 Sep 2023 20:08:08 +0100
Subject: [PATCH] fs: fix readdir and opendir recursive with unknown file types

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: https://github.com/nodejs/node/pull/49603
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
---
 lib/internal/fs/dir.js                       |  9 +++++----
 lib/internal/fs/utils.js                     |  2 +-
 test/sequential/test-fs-opendir-recursive.js | 17 ++++++++++-------
 test/sequential/test-fs-readdir-recursive.js | 17 ++++++++++-------
 4 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js
index ec0562843d5f5c..a443532d35d3ed 100644
--- 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
index 6e6c7ee58cf5d1..2fc7bf61e9c488 100644
--- 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
index 935be957e903c2..a7e9b9a318e5fb 100644
--- 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
index 1bb69f203ea526..2775573837786d 100644
--- 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