Skip to content

Commit

Permalink
fs: allow empty string for temp directory prefix
Browse files Browse the repository at this point in the history
The `fs` lib module's `mkdtemp()` and `mkdtempSync()` methods were
missing a validator, and weren't allowing the empty string as a valid
prefix.

PR-URL: #39028
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
  • Loading branch information
VoltrexKeyva authored and targos committed Jul 11, 2021
1 parent fe1c81f commit 83f3b95
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 12 deletions.
11 changes: 11 additions & 0 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,10 @@ rejection only when `recursive` is false.
### `fsPromises.mkdtemp(prefix[, options])`
<!-- YAML
added: v10.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/39028
description: The `prefix` parameter now accepts an empty string.
-->
* `prefix` {string}
Expand Down Expand Up @@ -2572,6 +2576,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/39028
description: The `prefix` parameter now accepts an empty string.
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/12562
description: The `callback` parameter is no longer optional. Not passing
Expand Down Expand Up @@ -4501,6 +4508,10 @@ See the POSIX mkdir(2) documentation for more details.
### `fs.mkdtempSync(prefix[, options])`
<!-- YAML
added: v5.10.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/39028
description: The `prefix` parameter now accepts an empty string.
-->
* `prefix` {string}
Expand Down
12 changes: 5 additions & 7 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ const {
codes: {
ERR_FS_FILE_TOO_LARGE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_ARG_TYPE,
ERR_FEATURE_UNAVAILABLE_ON_PLATFORM,
},
AbortError,
Expand Down Expand Up @@ -136,6 +135,7 @@ const {
validateEncoding,
validateFunction,
validateInteger,
validateString,
} = require('internal/validators');

const watchers = require('internal/fs/watchers');
Expand Down Expand Up @@ -2712,9 +2712,8 @@ realpath.native = (path, options, callback) => {
function mkdtemp(prefix, options, callback) {
callback = makeCallback(typeof options === 'function' ? options : callback);
options = getOptions(options, {});
if (!prefix || typeof prefix !== 'string') {
throw new ERR_INVALID_ARG_TYPE('prefix', 'string', prefix);
}

validateString(prefix, 'prefix');
nullCheck(prefix, 'prefix');
warnOnNonPortableTemplate(prefix);
const req = new FSReqCallback();
Expand All @@ -2730,9 +2729,8 @@ function mkdtemp(prefix, options, callback) {
*/
function mkdtempSync(prefix, options) {
options = getOptions(options, {});
if (!prefix || typeof prefix !== 'string') {
throw new ERR_INVALID_ARG_TYPE('prefix', 'string', prefix);
}

validateString(prefix, 'prefix');
nullCheck(prefix, 'prefix');
warnOnNonPortableTemplate(prefix);
const path = `${prefix}XXXXXX`;
Expand Down
7 changes: 3 additions & 4 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const { Buffer } = require('buffer');
const {
codes: {
ERR_FS_FILE_TOO_LARGE,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_METHOD_NOT_IMPLEMENTED,
},
Expand Down Expand Up @@ -72,6 +71,7 @@ const {
validateBuffer,
validateEncoding,
validateInteger,
validateString,
} = require('internal/validators');
const pathModule = require('path');
const { promisify } = require('internal/util');
Expand Down Expand Up @@ -693,9 +693,8 @@ async function realpath(path, options) {

async function mkdtemp(prefix, options) {
options = getOptions(options, {});
if (!prefix || typeof prefix !== 'string') {
throw new ERR_INVALID_ARG_TYPE('prefix', 'string', prefix);
}

validateString(prefix, 'prefix');
nullCheck(prefix);
warnOnNonPortableTemplate(prefix);
return binding.mkdtemp(`${prefix}XXXXXX`, options.encoding, kUsePromises);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-mkdtemp-prefix-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const common = require('../common');
const assert = require('assert');
const fs = require('fs');

const prefixValues = [undefined, null, 0, true, false, 1, ''];
const prefixValues = [undefined, null, 0, true, false, 1];

function fail(value) {
assert.throws(
Expand Down

0 comments on commit 83f3b95

Please sign in to comment.