Skip to content

Commit

Permalink
fs: warn on non-portable mkdtemp() templates
Browse files Browse the repository at this point in the history
Refs: #26435
PR-URL: #26980
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
cjihrig committed Apr 9, 2019
1 parent d11c4be commit b925379
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 3 deletions.
5 changes: 4 additions & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ const {
toUnixTimestamp,
validateOffsetLengthRead,
validateOffsetLengthWrite,
validatePath
validatePath,
warnOnNonPortableTemplate
} = require('internal/fs/utils');
const {
CHAR_FORWARD_SLASH,
Expand Down Expand Up @@ -1721,6 +1722,7 @@ function mkdtemp(prefix, options, callback) {
throw new ERR_INVALID_ARG_TYPE('prefix', 'string', prefix);
}
nullCheck(prefix, 'prefix');
warnOnNonPortableTemplate(prefix);
const req = new FSReqCallback();
req.oncomplete = callback;
binding.mkdtemp(`${prefix}XXXXXX`, options.encoding, req);
Expand All @@ -1733,6 +1735,7 @@ function mkdtempSync(prefix, options) {
throw new ERR_INVALID_ARG_TYPE('prefix', 'string', prefix);
}
nullCheck(prefix, 'prefix');
warnOnNonPortableTemplate(prefix);
const path = `${prefix}XXXXXX`;
const ctx = { path };
const result = binding.mkdtemp(path, options.encoding,
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ const {
toUnixTimestamp,
validateOffsetLengthRead,
validateOffsetLengthWrite,
validatePath
validatePath,
warnOnNonPortableTemplate
} = require('internal/fs/utils');
const {
parseMode,
Expand Down Expand Up @@ -461,6 +462,7 @@ async function mkdtemp(prefix, options) {
throw new ERR_INVALID_ARG_TYPE('prefix', 'string', prefix);
}
nullCheck(prefix);
warnOnNonPortableTemplate(prefix);
return binding.mkdtemp(`${prefix}XXXXXX`, options.encoding, kUsePromises);
}

Expand Down
15 changes: 14 additions & 1 deletion lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,18 @@ const validatePath = hideStackFrames((path, propName = 'path') => {
}
});

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 && template.endsWith('X')) {
process.emitWarning('mkdtemp() templates ending with X are not portable. ' +
'For details see: https://nodejs.org/api/fs.html');
nonPortableTemplateWarn = false;
}
}

module.exports = {
assertEncoding,
copyObject,
Expand All @@ -448,5 +460,6 @@ module.exports = {
toUnixTimestamp,
validateOffsetLengthRead,
validateOffsetLengthWrite,
validatePath
validatePath,
warnOnNonPortableTemplate
};
5 changes: 5 additions & 0 deletions test/parallel/test-fs-mkdtemp.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,8 @@ 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));

0 comments on commit b925379

Please sign in to comment.