fs: return Buffer from mkdtemp when prefix is a Buffer

Signed-off-by: Hamid Reza Ghavami <hamidr.ghavami@gmail.com>
PR-URL: #64397
Fixes: #58795
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Author: hamidrezaghavami

doc/api/fs.md

@@ -1637,6 +1637,9 @@ makeDirectory().catch(console.error);
 <!-- YAML
 added: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64397
+    description: The `encoding` option now defaults to `'buffer'` if `prefix` is a `Buffer`.
   - version:
     - v20.6.0
     - v18.19.0
@@ -1651,7 +1654,7 @@ changes:
 
 * `prefix` {string|Buffer|URL}
 * `options` {string|Object}
-  * `encoding` {string} **Default:** `'utf8'`
+  * `encoding` {string} **Default:** `'utf8'` (or `'buffer'` if `prefix` is a `Buffer`)
 * Returns: {Promise} Fulfills with the created directory path.
   If `encoding` is `'buffer'`, then the resulting directory
   path is returned as a {Buffer}. Otherwise, the path is returned as a
@@ -1688,11 +1691,15 @@ characters directly to the `prefix` string. For instance, given a directory
 
 <!-- YAML
 added: v24.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64397
+    description: The `encoding` option now defaults to `'buffer'` if `prefix` is a `Buffer`.
 -->
 
 * `prefix` {string|Buffer|URL}
 * `options` {string|Object}
-  * `encoding` {string} **Default:** `'utf8'`
+  * `encoding` {string} **Default:** `'utf8'` (or `'buffer'` if `prefix` is a `Buffer`)
 * Returns: {Promise} Fulfills with a Promise for an async-disposable Object:
   * `path` {string|Buffer} The path of the created directory.
   * `remove` {AsyncFunction} A function which removes the created directory.
@@ -3978,6 +3985,9 @@ See the POSIX mkdir(2) documentation for more details.
 <!-- YAML
 added: v5.10.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64397
+    description: The `encoding` option now defaults to `'buffer'` if `prefix` is a `Buffer`.
   - version:
     - v20.6.0
     - v18.19.0
@@ -4008,7 +4018,7 @@ changes:
 
 * `prefix` {string|Buffer|URL}
 * `options` {string|Object}
-  * `encoding` {string} **Default:** `'utf8'`
+  * `encoding` {string} **Default:** `'utf8'` (or `'buffer'` if `prefix` is a `Buffer`)
 * `callback` {Function}
   * `err` {Error}
   * `directory` {string|Buffer}
@@ -6490,6 +6500,9 @@ See the POSIX mkdir(2) documentation for more details.
 <!-- YAML
 added: v5.10.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64397
+    description: The `encoding` option now defaults to `'buffer'` if `prefix` is a `Buffer`.
   - version:
     - v20.6.0
     - v18.19.0
@@ -6504,7 +6517,7 @@ changes:
 
 * `prefix` {string|Buffer|URL}
 * `options` {string|Object}
-  * `encoding` {string} **Default:** `'utf8'`
+  * `encoding` {string} **Default:** `'utf8'` (or `'buffer'` if `prefix` is a `Buffer`)
 * Returns: {string|Buffer}
 
 Returns the created directory path. If `encoding` is `'buffer'`, then the
@@ -6521,11 +6534,15 @@ object with an `encoding` property specifying the character encoding to use.
 
 <!-- YAML
 added: v24.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64397
+    description: The `encoding` option now defaults to `'buffer'` if `prefix` is a `Buffer`.
 -->
 
 * `prefix` {string|Buffer|URL}
 * `options` {string|Object}
-  * `encoding` {string} **Default:** `'utf8'`
+  * `encoding` {string} **Default:** `'utf8'` (or `'buffer'` if `prefix` is a `Buffer`)
 * Returns: {Object} A disposable object:
   * `path` {string|Buffer} The path of the created directory.
   * `remove` {Function} A function which removes the created directory.

lib/fs.js

@@ -3683,7 +3683,9 @@ function mkdtemp(prefix, options, callback) {
   if (h !== null && vfsResult(h.mkdtemp(prefix, typeof options === 'function' ? undefined : options), callback)) return;
 
   options = getOptions(options);
-
+  if (BufferIsBuffer(prefix)) {
+    options = { ...options, encoding: 'buffer' };
+  }
   prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
 
@@ -3706,7 +3708,9 @@ function mkdtempSync(prefix, options) {
   }
 
   options = getOptions(options);
-
+  if (BufferIsBuffer(prefix)) {
+    options = { ...options, encoding: 'buffer' };
+  }
   prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
   return binding.mkdtemp(prefix, options.encoding);
@@ -3722,7 +3726,9 @@ function mkdtempSync(prefix, options) {
  */
 function mkdtempDisposableSync(prefix, options) {
   options = getOptions(options);
-
+  if (BufferIsBuffer(prefix)) {
+    options = { ...options, encoding: 'buffer' };
+  }
   prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
 

lib/internal/fs/promises.js

@@ -2025,9 +2025,10 @@ async function mkdtemp(prefix, options) {
     const promise = h.mkdtemp(prefix, options);
     if (promise !== undefined) return await promise;
   }
-
   options = getOptions(options);
-
+  if (BufferIsBuffer(prefix)) {
+    options = { ...options, encoding: 'buffer' };
+  }
   prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
 
@@ -2040,7 +2041,9 @@ async function mkdtemp(prefix, options) {
 
 async function mkdtempDisposable(prefix, options) {
   options = getOptions(options);
-
+  if (BufferIsBuffer(prefix)) {
+    options = { ...options, encoding: 'buffer' };
+  }
   prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
 

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

@@ -0,0 +1,26 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+
+const prefixString = path.join(tmpdir.path, 'buffer-');
+const prefixBuffer = Buffer.from(prefixString);
+
+// 1. Test Sync API
+const resultSync = fs.mkdtempSync(prefixBuffer);
+assert.strictEqual(Buffer.isBuffer(resultSync), true);
+
+// 2. Test Callback API
+fs.mkdtemp(prefixBuffer, common.mustSucceed((result) => {
+  assert.strictEqual(Buffer.isBuffer(result), true);
+}));
+
+// 3. Test Promises API
+fs.promises.mkdtemp(prefixBuffer)
+  .then(common.mustCall((resultPromise) => {
+    assert.strictEqual(Buffer.isBuffer(resultPromise), true);
+  }));

test/parallel/test-fs-mkdtemp.js

@@ -64,14 +64,13 @@ function handler(err, folder) {
 {
   const tmpFolder = fs.mkdtempSync(Buffer.from(tmpdir.resolve('foo.')));
 
-  assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length);
+  assert.strictEqual(path.basename(tmpFolder.toString()).length, 'foo.XXXXXX'.length);
   assert(fs.existsSync(tmpFolder));
 
   const utf8 = fs.mkdtempSync(Buffer.from(tmpdir.resolve('\u0222abc.')));
-  assert.strictEqual(Buffer.byteLength(path.basename(utf8)),
+  assert.strictEqual(Buffer.byteLength(path.basename(utf8.toString())),
                      Buffer.byteLength('\u0222abc.XXXXXX'));
   assert(fs.existsSync(utf8));
-
   fs.mkdtemp(Buffer.from(tmpdir.resolve('bar.')), common.mustCall(handler));
 
   // Same test as above, but making sure that passing an options object doesn't