fs: warn on non-portable mkdtemp() templates

Refs: #26435 PR-URL: #26980 Reviewed-By: James M Snell <[email protected]>
nodejs · Apr 9, 2019 · b925379 · b925379
1 parent d11c4be
commit b925379
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 3 deletions.
diff --git a/lib/fs.js b/lib/fs.js
@@ -73,7 +73,8 @@ const {
   toUnixTimestamp,
   validateOffsetLengthRead,
   validateOffsetLengthWrite,
-  validatePath
+  validatePath,
+  warnOnNonPortableTemplate
 } = require('internal/fs/utils');
 const {
   CHAR_FORWARD_SLASH,
@@ -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);
@@ -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,

diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -31,7 +31,8 @@ const {
   toUnixTimestamp,
   validateOffsetLengthRead,
   validateOffsetLengthWrite,
-  validatePath
+  validatePath,
+  warnOnNonPortableTemplate
 } = require('internal/fs/utils');
 const {
   parseMode,
@@ -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);
 }
 

diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js
@@ -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,
@@ -448,5 +460,6 @@ module.exports = {
   toUnixTimestamp,
   validateOffsetLengthRead,
   validateOffsetLengthWrite,
-  validatePath
+  validatePath,
+  warnOnNonPortableTemplate
 };
diff --git a/test/parallel/test-fs-mkdtemp.js b/test/parallel/test-fs-mkdtemp.js
@@ -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));