Skip to content

Commit

Permalink
fs: make mkdtemp accept buffers and URL
Browse files Browse the repository at this point in the history
PR-URL: #48828
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
LiviaMedeiros authored and RafaelGSS committed Aug 17, 2023
1 parent bc009d0 commit d634d78
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 35 deletions.
15 changes: 12 additions & 3 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1159,14 +1159,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
Expand Down Expand Up @@ -3244,6 +3247,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
Expand All @@ -3267,7 +3273,7 @@ changes:
description: The `callback` parameter is optional now.
-->
* `prefix` {string}
* `prefix` {string|Buffer|URL}
* `options` {string|Object}
* `encoding` {string} **Default:** `'utf8'`
* `callback` {Function}
Expand Down Expand Up @@ -5550,14 +5556,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}
Expand Down
28 changes: 19 additions & 9 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ const {
getValidatedPath,
getValidMode,
handleErrorFromBinding,
nullCheck,
possiblyTransformPath,
preprocessSymlinkDestination,
Stats,
Expand Down Expand Up @@ -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,
Expand All @@ -2912,29 +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);
Expand Down
14 changes: 10 additions & 4 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ const {
getStatsFromBinding,
getValidatedPath,
getValidMode,
nullCheck,
preprocessSymlinkDestination,
stringToFlags,
stringToSymlinkType,
Expand Down Expand Up @@ -996,10 +995,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) {
Expand Down
5 changes: 4 additions & 1 deletion lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const {
StringPrototypeEndsWith,
StringPrototypeIncludes,
Symbol,
TypedArrayPrototypeAt,
TypedArrayPrototypeIncludes,
} = primordials;

Expand Down Expand Up @@ -751,7 +752,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;
Expand Down
110 changes: 92 additions & 18 deletions test/parallel/test-fs-mkdtemp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

0 comments on commit d634d78

Please sign in to comment.