fs: make mkdtemp accept buffers and URL

PR-URL: nodejs#48828 Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Ceres6 · Aug 14, 2023 · 6bd6214 · 6bd6214
1 parent 7a8bb62
commit 6bd6214
Show file tree

Hide file tree

Showing 5 changed files with 139 additions and 35 deletions.
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -1163,14 +1163,17 @@ makeDirectory().catch(console.error);
 <!-- YAML
 added: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/48828
+    description: The `prefix` parameter now accepts buffers and URL.
   - version:
       - v16.5.0
       - v14.18.0
     pr-url: https://github.com/nodejs/node/pull/39028
     description: The `prefix` parameter now accepts an empty string.
 -->
 
-* `prefix` {string}
+* `prefix` {string|Buffer|URL}
 * `options` {string|Object}
   * `encoding` {string} **Default:** `'utf8'`
 * Returns: {Promise}  Fulfills with a string containing the file system path
@@ -3254,6 +3257,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/48828
+    description: The `prefix` parameter now accepts buffers and URL.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41678
     description: Passing an invalid callback to the `callback` argument
@@ -3277,7 +3283,7 @@ changes:
     description: The `callback` parameter is optional now.
 -->
 
-* `prefix` {string}
+* `prefix` {string|Buffer|URL}
 * `options` {string|Object}
   * `encoding` {string} **Default:** `'utf8'`
 * `callback` {Function}
@@ -5566,14 +5572,17 @@ 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/48828
+    description: The `prefix` parameter now accepts buffers and URL.
   - version:
       - v16.5.0
       - v14.18.0
     pr-url: https://github.com/nodejs/node/pull/39028
     description: The `prefix` parameter now accepts an empty string.
 -->
 
-* `prefix` {string}
+* `prefix` {string|Buffer|URL}
 * `options` {string|Object}
   * `encoding` {string} **Default:** `'utf8'`
 * Returns: {string}

diff --git a/lib/fs.js b/lib/fs.js
@@ -105,7 +105,6 @@ const {
   getValidatedPath,
   getValidMode,
   handleErrorFromBinding,
-  nullCheck,
   possiblyTransformPath,
   preprocessSymlinkDestination,
   Stats,
@@ -2900,7 +2899,7 @@ realpath.native = (path, options, callback) => {
 
 /**
  * Creates a unique temporary directory.
- * @param {string} prefix
+ * @param {string | Buffer | URL} prefix
  * @param {string | { encoding?: string; }} [options]
  * @param {(
  *   err?: Error,
@@ -2912,27 +2911,40 @@ function mkdtemp(prefix, options, callback) {
   callback = makeCallback(typeof options === 'function' ? options : callback);
   options = getOptions(options);
 
-  validateString(prefix, 'prefix');
-  nullCheck(prefix, 'prefix');
+  prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
+
+  let path;
+  if (typeof prefix === 'string') {
+    path = `${prefix}XXXXXX`;
+  } else {
+    path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
+  }
+
   const req = new FSReqCallback();
   req.oncomplete = callback;
-  binding.mkdtemp(`${prefix}XXXXXX`, options.encoding, req);
+  binding.mkdtemp(path, options.encoding, req);
 }
 
 /**
  * Synchronously creates a unique temporary directory.
- * @param {string} prefix
+ * @param {string | Buffer | URL} prefix
  * @param {string | { encoding?: string; }} [options]
  * @returns {string}
  */
 function mkdtempSync(prefix, options) {
   options = getOptions(options);
 
-  validateString(prefix, 'prefix');
-  nullCheck(prefix, 'prefix');
+  prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
-  const path = `${prefix}XXXXXX`;
+
+  let path;
+  if (typeof prefix === 'string') {
+    path = `${prefix}XXXXXX`;
+  } else {
+    path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
+  }
+
   const ctx = { path };
   const result = binding.mkdtemp(path, options.encoding,
                                  undefined, ctx);

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -59,7 +59,6 @@ const {
   getStatsFromBinding,
   getValidatedPath,
   getValidMode,
-  nullCheck,
   preprocessSymlinkDestination,
   stringToFlags,
   stringToSymlinkType,
@@ -997,10 +996,17 @@ async function realpath(path, options) {
 async function mkdtemp(prefix, options) {
   options = getOptions(options);
 
-  validateString(prefix, 'prefix');
-  nullCheck(prefix);
+  prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
-  return binding.mkdtemp(`${prefix}XXXXXX`, options.encoding, kUsePromises);
+
+  let path;
+  if (typeof prefix === 'string') {
+    path = `${prefix}XXXXXX`;
+  } else {
+    path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
+  }
+
+  return binding.mkdtemp(path, options.encoding, kUsePromises);
 }
 
 async function writeFile(path, data, options) {

diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
@@ -20,6 +20,7 @@ const {
   StringPrototypeEndsWith,
   StringPrototypeIncludes,
   Symbol,
+  TypedArrayPrototypeAt,
   TypedArrayPrototypeIncludes,
 } = primordials;
 
@@ -749,7 +750,9 @@ let nonPortableTemplateWarn = true;
 function warnOnNonPortableTemplate(template) {
   // Template strings passed to the mkdtemp() family of functions should not
   // end with 'X' because they are handled inconsistently across platforms.
-  if (nonPortableTemplateWarn && StringPrototypeEndsWith(template, 'X')) {
+  if (nonPortableTemplateWarn &&
+    ((typeof template === 'string' && StringPrototypeEndsWith(template, 'X')) ||
+    (typeof template !== 'string' && TypedArrayPrototypeAt(template, -1) === 0x58))) {
     process.emitWarning('mkdtemp() templates ending with X are not portable. ' +
                         'For details see: https://nodejs.org/api/fs.html');
     nonPortableTemplateWarn = false;

diff --git a/test/parallel/test-fs-mkdtemp.js b/test/parallel/test-fs-mkdtemp.js
@@ -8,29 +8,103 @@ const path = require('path');
 const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
 
-const tmpFolder = fs.mkdtempSync(path.join(tmpdir.path, 'foo.'));
-
-assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length);
-assert(fs.existsSync(tmpFolder));
-
-const utf8 = fs.mkdtempSync(path.join(tmpdir.path, '\u0222abc.'));
-assert.strictEqual(Buffer.byteLength(path.basename(utf8)),
-                   Buffer.byteLength('\u0222abc.XXXXXX'));
-assert(fs.existsSync(utf8));
-
 function handler(err, folder) {
   assert.ifError(err);
   assert(fs.existsSync(folder));
   assert.strictEqual(this, undefined);
 }
 
-fs.mkdtemp(path.join(tmpdir.path, 'bar.'), common.mustCall(handler));
+// Test with plain string
+{
+  const tmpFolder = fs.mkdtempSync(path.join(tmpdir.path, 'foo.'));
+
+  assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length);
+  assert(fs.existsSync(tmpFolder));
+
+  const utf8 = fs.mkdtempSync(path.join(tmpdir.path, '\u0222abc.'));
+  assert.strictEqual(Buffer.byteLength(path.basename(utf8)),
+                     Buffer.byteLength('\u0222abc.XXXXXX'));
+  assert(fs.existsSync(utf8));
+
+  fs.mkdtemp(path.join(tmpdir.path, 'bar.'), common.mustCall(handler));
+
+  // Same test as above, but making sure that passing an options object doesn't
+  // affect the way the callback function is handled.
+  fs.mkdtemp(path.join(tmpdir.path, 'bar.'), {}, common.mustCall(handler));
+
+  const warningMsg = 'mkdtemp() templates ending with X are not portable. ' +
+                     'For details see: https://nodejs.org/api/fs.html';
+  common.expectWarning('Warning', warningMsg);
+  fs.mkdtemp(path.join(tmpdir.path, 'bar.X'), common.mustCall(handler));
+}
+
+// Test with URL object
+{
+  tmpdir.url = new URL(`file://${tmpdir.path}`);
+  const urljoin = (base, path) => new URL(path, base);
+
+  const tmpFolder = fs.mkdtempSync(urljoin(tmpdir.url, 'foo.'));
+
+  assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length);
+  assert(fs.existsSync(tmpFolder));
+
+  const utf8 = fs.mkdtempSync(urljoin(tmpdir.url, '\u0222abc.'));
+  assert.strictEqual(Buffer.byteLength(path.basename(utf8)),
+                     Buffer.byteLength('\u0222abc.XXXXXX'));
+  assert(fs.existsSync(utf8));
+
+  fs.mkdtemp(urljoin(tmpdir.url, 'bar.'), common.mustCall(handler));
+
+  // Same test as above, but making sure that passing an options object doesn't
+  // affect the way the callback function is handled.
+  fs.mkdtemp(urljoin(tmpdir.url, 'bar.'), {}, common.mustCall(handler));
+
+  // Warning fires only once
+  fs.mkdtemp(urljoin(tmpdir.url, 'bar.X'), common.mustCall(handler));
+}
+
+// Test with Buffer
+{
+  const tmpFolder = fs.mkdtempSync(Buffer.from(path.join(tmpdir.path, 'foo.')));
+
+  assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length);
+  assert(fs.existsSync(tmpFolder));
 
-// Same test as above, but making sure that passing an options object doesn't
-// affect the way the callback function is handled.
-fs.mkdtemp(path.join(tmpdir.path, 'bar.'), {}, common.mustCall(handler));
+  const utf8 = fs.mkdtempSync(Buffer.from(path.join(tmpdir.path, '\u0222abc.')));
+  assert.strictEqual(Buffer.byteLength(path.basename(utf8)),
+                     Buffer.byteLength('\u0222abc.XXXXXX'));
+  assert(fs.existsSync(utf8));
+
+  fs.mkdtemp(Buffer.from(path.join(tmpdir.path, 'bar.')), common.mustCall(handler));
+
+  // Same test as above, but making sure that passing an options object doesn't
+  // affect the way the callback function is handled.
+  fs.mkdtemp(Buffer.from(path.join(tmpdir.path, 'bar.')), {}, common.mustCall(handler));
+
+  // Warning fires only once
+  fs.mkdtemp(Buffer.from(path.join(tmpdir.path, 'bar.X')), common.mustCall(handler));
+}
 
-const warningMsg = 'mkdtemp() templates ending with X are not portable. ' +
-                   'For details see: https://nodejs.org/api/fs.html';
-common.expectWarning('Warning', warningMsg);
-fs.mkdtemp(path.join(tmpdir.path, 'bar.X'), common.mustCall(handler));
+// Test with Uint8Array
+{
+  const encoder = new TextEncoder();
+
+  const tmpFolder = fs.mkdtempSync(encoder.encode(path.join(tmpdir.path, 'foo.')));
+
+  assert.strictEqual(path.basename(tmpFolder).length, 'foo.XXXXXX'.length);
+  assert(fs.existsSync(tmpFolder));
+
+  const utf8 = fs.mkdtempSync(encoder.encode(path.join(tmpdir.path, '\u0222abc.')));
+  assert.strictEqual(Buffer.byteLength(path.basename(utf8)),
+                     Buffer.byteLength('\u0222abc.XXXXXX'));
+  assert(fs.existsSync(utf8));
+
+  fs.mkdtemp(encoder.encode(path.join(tmpdir.path, 'bar.')), common.mustCall(handler));
+
+  // Same test as above, but making sure that passing an options object doesn't
+  // affect the way the callback function is handled.
+  fs.mkdtemp(encoder.encode(path.join(tmpdir.path, 'bar.')), {}, common.mustCall(handler));
+
+  // Warning fires only once
+  fs.mkdtemp(encoder.encode(path.join(tmpdir.path, 'bar.X')), common.mustCall(handler));
+}