fix: keep tmpfile names from exceeding the destination name

README.md

@@ -31,8 +31,7 @@ var writeFileAtomic = require('write-file-atomic')
 writeFileAtomic(filename, data, [options], [callback])
 ```
 
-The file is initially named `filename + "." + murmurhex(__filename, process.pid, ++invocations)`.
-Note that `require('worker_threads').threadId` is used in addition to `process.pid` if running inside of a worker thread.
+The file is initially written to a temporary name in the same directory. That name is the destination plus a unique numeric suffix from a hash of this module's path, `process.pid`, worker `threadId` (when running in a worker thread), and an invocation counter. If the destination basename is longer than 11 characters (the longest `'.'`+uint32 suffix), the suffix replaces the end of the basename instead, so a destination name that is legal on the filesystem cannot produce `ENAMETOOLONG` for the tmpfile.
 If writeFile completes successfully then, if passed the **chown** option it will change
 the ownership of the file. Finally it renames the file back to the filename you specified. If
 it encounters errors at any of these steps it will attempt to unlink the temporary file and then

lib/index.js

@@ -27,14 +27,21 @@ const threadId = (function getId () {
 
 let invocations = 0
 function getTmpname (filename) {
-  return filename + '.' +
+  const suffix = '.' +
     crypto.createHash('sha1')
       .update(__filename)
       .update(String(process.pid))
       .update(String(threadId))
       .update(String(++invocations))
       .digest()
       .readUInt32BE(0)
+  // Never lengthen the basename: appending overflows NAME_MAX on long names (#63).
+  // uint32 suffix is 2-11 chars; use the max so the choice does not depend on hash width.
+  const basename = path.basename(filename)
+  if (basename.length <= 11) {
+    return filename + suffix
+  }
+  return filename.slice(0, filename.length - suffix.length) + suffix
 }
 
 function cleanupOnExit (tmpfile) {

test/basic.js

@@ -161,6 +161,19 @@ t.test('getTmpname', t => {
   const a = getTmpname('abc.def')
   const b = getTmpname('abc.def')
   t.not(a, b, 'different invocations of getTmpname get different results')
+  t.match(a, /^abc\.def\.\d+$/, 'short names keep an appended unique suffix')
+
+  const longBase = 'a'.repeat(255)
+  const longFile = '/tmp/' + longBase
+  const longTmp = getTmpname(longFile)
+  t.equal(
+    longTmp.length,
+    longFile.length,
+    'tmp path is no longer than the destination when the basename is already long'
+  )
+  t.equal(longTmp.slice(0, '/tmp/'.length), '/tmp/', 'tmp stays in the destination directory')
+  t.match(longTmp, /\.\d+$/, 'tmp still ends with a unique numeric suffix')
+  t.not(getTmpname(longFile), longTmp, 'long names still get unique tmp names')
   t.end()
 })
 
@@ -226,7 +239,7 @@ t.test('async tests', t => {
     writeFileAtomic('norename', 'test', err => {
       t.equal(err && err.message, 'ENORENAME', 'Rename errors propagate')
     })
-    writeFileAtomic('norename nounlink', 'test', err => {
+    writeFileAtomic('norename nounlink ' + 'x'.repeat(32), 'test', err => {
       t.equal(err && err.message, 'ENORENAME',
         'Failure to unlink the temp file does not clobber the original error')
     })
@@ -352,7 +365,7 @@ t.test('sync tests', t => {
     })
     throws(t, 'ENORENAME',
       'Failure to unlink the temp file does not clobber the original error', () => {
-        writeFileAtomicSync('norename nounlink', 'test')
+        writeFileAtomicSync('norename nounlink ' + 'x'.repeat(32), 'test')
       })
     throws(t, 'ENOFSYNC', 'Fsync errors propagate', () => {
       writeFileAtomicSync('nofsync', 'test')

test/integration.js

@@ -305,6 +305,34 @@ t.test('does not change chown by default (sync)', t => {
   didWriteFileAtomicSync(t, { uid: 42, gid: 43 }, file, '44')
 })
 
+t.test('writes a file at the filesystem name length limit (sync)', t => {
+  const writeFileAtomic = require('..')
+  const file = path.join(workdir, 'x'.repeat(255))
+  let tmpfile
+  writeFileAtomic.sync(file, '42', {
+    tmpfileCreated (tmp) {
+      tmpfile = tmp
+    },
+  })
+  t.equal(path.basename(tmpfile).length, 255, 'tmp basename is no longer than the target')
+  t.equal(readFile(file), '42', 'content ok')
+  t.end()
+})
+
+t.test('writes a file at the filesystem name length limit (async)', t => {
+  const writeFileAtomic = require('..')
+  const file = path.join(workdir, 'y'.repeat(255))
+  writeFileAtomic(file, '43', {
+    tmpfileCreated (tmpfile) {
+      t.equal(path.basename(tmpfile).length, 255, 'tmp basename is no longer than the target')
+    },
+  }, err => {
+    t.error(err, 'no error')
+    t.equal(readFile(file), '43', 'content ok')
+    t.end()
+  })
+})
+
 t.test('cleanup', t => {
   fs.rmSync(workdir, { recursive: true, force: true })
   t.end()