Conversation
Appending a unique suffix to a basename that already saturates the filesystem NAME_MAX made the tmpfile unopenable (ENAMETOOLONG). Replace the end of the basename with the suffix so a legal destination name is also a legal tmp name, including on filesystems whose NAME_MAX is not 255. Fixes npm#63
Comparing against the actual uint32 decimal width made names of length 3-11 append or truncate depending on the hash. Use the maximum width (11) so the choice is stable, while still never lengthening names that can hit NAME_MAX.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getTmpnameno longer appends a suffix that can push a legal destination basename past the filesystemNAME_MAX. If the destination basename is longer than 11 characters (the longest'.'+ uint32 suffix), that unique suffix replaces the end of the basename so the tmpfile is not longer than the target. Names of 11 characters or fewer still getfilename.<hash>, as before. Covers both the async and sync writers; they already sharegetTmpname.Fixes #63. The request was that the temp name be "short or at least not longer than the target file name." The linked pnpm report (pnpm/pnpm#2605) overflowed on eCryptfs, whose
NAME_MAXis 143, not 255.What I chose and the alternative
Never lengthen a tmpfile basename that is already longer than the longest suffix. Same unique suffix as today (hash of this module's path,
process.pid, workerthreadId, invocation counter). Decide append-vs-replace using 11, not this invocation's decimal width, so a name likeindex.json(10) is not sometimes appended and sometimes truncated.Alternatives: truncate only when the tmp basename would exceed 255; or use a short same-directory
.$hashname (floated on PR #3); or retryopenonENAMETOOLONG.A hardcoded 255 would not have fixed the eCryptfs case in the original report. A non-unique
.tmpsuffix was already rejected on PR #3. Retrying onENAMETOOLONGwould preservepackage.json.<n>for typical files but is more code on both the sync and async paths. Node has no portable per-filesystemNAME_MAX.I can switch to "truncate only past 255", to a short
.$hashtmp name, or to.update(filename)in the hash, if you prefer.Test plan
npx tap test/basic.js:getTmpnamestill unique; short names still matchfilename.{digits}; a 255-character basename produces a tmp path of equal length in the same directorynpx tap test/integration.js: sync and async writes of a 255-character basename succeed;tmpfileCreatedreports a basename of length 255getTmpnameto always append: the new tests fail withENAMETOOLONG/ a longer tmp path