fs: fix readdir failure when libuv returns UV_DIRENT_UNKNOWN

Fixes: #33348

PR-URL: #33395
Refs: #33348
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: shackijj

lib/internal/fs/utils.js

@@ -173,6 +173,31 @@ function copyObject(source) {
   return target;
 }
 
+const bufferSep = Buffer.from(pathModule.sep);
+
+function join(path, name) {
+  if ((typeof path === 'string' || isUint8Array(path)) &&
+      name === undefined) {
+    return path;
+  }
+
+  if (typeof path === 'string' && isUint8Array(name)) {
+    const pathBuffer = Buffer.from(pathModule.join(path, pathModule.sep));
+    return Buffer.concat([pathBuffer, name]);
+  }
+
+  if (typeof path === 'string' && typeof name === 'string') {
+    return pathModule.join(path, name);
+  }
+
+  if (isUint8Array(path) && isUint8Array(name)) {
+    return Buffer.concat([path, bufferSep, name]);
+  }
+
+  throw new ERR_INVALID_ARG_TYPE(
+    'path', ['string', 'Buffer'], path);
+}
+
 function getDirents(path, [names, types], callback) {
   let i;
   if (typeof callback === 'function') {
@@ -185,7 +210,14 @@ function getDirents(path, [names, types], callback) {
         const name = names[i];
         const idx = i;
         toFinish++;
-        lazyLoadFs().lstat(pathModule.join(path, name), (err, stats) => {
+        let filepath;
+        try {
+          filepath = join(path, name);
+        } catch (err) {
+          callback(err);
+          return;
+        }
+        lazyLoadFs().lstat(filepath, (err, stats) => {
           if (err) {
             callback(err);
             return;
@@ -214,7 +246,14 @@ function getDirents(path, [names, types], callback) {
 function getDirent(path, name, type, callback) {
   if (typeof callback === 'function') {
     if (type === UV_DIRENT_UNKNOWN) {
-      lazyLoadFs().lstat(pathModule.join(path, name), (err, stats) => {
+      let filepath;
+      try {
+        filepath = join(path, name);
+      } catch (err) {
+        callback(err);
+        return;
+      }
+      lazyLoadFs().lstat(filepath, (err, stats) => {
         if (err) {
           callback(err);
           return;
@@ -225,7 +264,7 @@ function getDirent(path, name, type, callback) {
       callback(null, new Dirent(name, type));
     }
   } else if (type === UV_DIRENT_UNKNOWN) {
-    const stats = lazyLoadFs().lstatSync(pathModule.join(path, name));
+    const stats = lazyLoadFs().lstatSync(join(path, name));
     return new DirentFromStats(name, stats);
   } else {
     return new Dirent(name, type);

test/parallel/test-fs-readdir-buffer.js

@@ -0,0 +1,17 @@
+'use strict';
+const common = require('../common');
+const fs = require('fs');
+
+if (!common.isOSX) {
+  common.skip('this tests works only on MacOS');
+}
+
+const assert = require('assert');
+
+fs.readdir(
+  Buffer.from('/dev'),
+  { withFileTypes: true, encoding: 'buffer' },
+  common.mustCall((e, d) => {
+    assert.strictEqual(e, null);
+  })
+);

test/parallel/test-fs-utils-get-dirents.js

@@ -0,0 +1,123 @@
+// Flags: --expose-internals
+'use strict';
+
+const common = require('../common');
+const { getDirents, getDirent } = require('internal/fs/utils');
+const assert = require('assert');
+const { internalBinding } = require('internal/test/binding');
+const { UV_DIRENT_UNKNOWN } = internalBinding('constants').fs;
+const fs = require('fs');
+const path = require('path');
+
+const tmpdir = require('../common/tmpdir');
+const filename = 'foo';
+
+{
+  // setup
+  tmpdir.refresh();
+  fs.writeFileSync(path.join(tmpdir.path, filename), '');
+}
+// getDirents
+{
+  // string + string
+  getDirents(
+    tmpdir.path,
+    [[filename], [UV_DIRENT_UNKNOWN]],
+    common.mustCall((err, names) => {
+      assert.strictEqual(err, null);
+      assert.strictEqual(names.length, 1);
+    },
+    ));
+}
+{
+  // string + Buffer
+  getDirents(
+    tmpdir.path,
+    [[Buffer.from(filename)], [UV_DIRENT_UNKNOWN]],
+    common.mustCall((err, names) => {
+      assert.strictEqual(err, null);
+      assert.strictEqual(names.length, 1);
+    },
+    ));
+}
+{
+  // Buffer + Buffer
+  getDirents(
+    Buffer.from(tmpdir.path),
+    [[Buffer.from(filename)], [UV_DIRENT_UNKNOWN]],
+    common.mustCall((err, names) => {
+      assert.strictEqual(err, null);
+      assert.strictEqual(names.length, 1);
+    },
+    ));
+}
+{
+  // wrong combination
+  getDirents(
+    42,
+    [[Buffer.from(filename)], [UV_DIRENT_UNKNOWN]],
+    common.mustCall((err) => {
+      assert.strictEqual(
+        err.message,
+        [
+          'The "path" argument must be of type string or an ' +
+          'instance of Buffer. Received type number (42)'
+        ].join(''));
+    },
+    ));
+}
+// getDirent
+{
+  // string + string
+  getDirent(
+    tmpdir.path,
+    filename,
+    UV_DIRENT_UNKNOWN,
+    common.mustCall((err, dirent) => {
+      assert.strictEqual(err, null);
+      assert.strictEqual(dirent.name, filename);
+    },
+    ));
+}
+{
+  // string + Buffer
+  const filenameBuffer = Buffer.from(filename);
+  getDirent(
+    tmpdir.path,
+    filenameBuffer,
+    UV_DIRENT_UNKNOWN,
+    common.mustCall((err, dirent) => {
+      assert.strictEqual(err, null);
+      assert.strictEqual(dirent.name, filenameBuffer);
+    },
+    ));
+}
+{
+  // Buffer + Buffer
+  const filenameBuffer = Buffer.from(filename);
+  getDirent(
+    Buffer.from(tmpdir.path),
+    filenameBuffer,
+    UV_DIRENT_UNKNOWN,
+    common.mustCall((err, dirent) => {
+      assert.strictEqual(err, null);
+      assert.strictEqual(dirent.name, filenameBuffer);
+    },
+    ));
+}
+{
+  // wrong combination
+  getDirent(
+    42,
+    Buffer.from(filename),
+    UV_DIRENT_UNKNOWN,
+    common.mustCall((err) => {
+      assert.strictEqual(
+        err.message,
+        [
+          'The "path" argument must be of type string or an ' +
+          'instance of Buffer. Received type number (42)'
+        ].join(''));
+    },
+    ));
+}