镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fs: fix mkdtempDisposable with Buffer paths - #66594

Open
gengjiawen wants to merge 1 commit into
nodejs:mainfrom
gengjiawen:fs-mkdtemp-disposable-buffer
Open

gengjiawen wants to merge 1 commit into
nodejs:mainfrom
gengjiawen:fs-mkdtemp-disposable-buffer

Conversation

@gengjiawen

Copy link
Copy Markdown
Member

fs.mkdtempDisposableSync() and fsPromises.mkdtempDisposable() resolve the path of the new directory with path.resolve(), so that remove() still works after process.chdir(). path.resolve() only accepts strings, so when the path is a Buffer (a Buffer prefix, or encoding: 'buffer'), both functions throw ERR_INVALID_ARG_TYPE after the directory has been created, and the directory is never removed.

absoluteMkdtempPath() in internal/fs/utils stashes that path for remove():

  • string paths are resolved exactly as before
  • on Windows, a Buffer path is UTF-8, so it is resolved with path.resolve() and encoded back. Root-relative (\foo) and drive-relative (D:foo) prefixes keep win32 semantics, and non-ASCII characters in the cwd stay intact
  • on POSIX, an absolute Buffer is kept byte for byte, including bytes that are not valid UTF-8. A relative Buffer gets the cwd from creation time prepended, via the existing join() used for other string-plus-Buffer paths

The tests cover both the sync and the promise API: a Buffer prefix, encoding: 'buffer', a relative Buffer prefix followed by process.chdir(), a relative Buffer prefix in a non-ASCII directory, on Linux a prefix containing a byte that is not valid UTF-8, and on POSIX a .. component after a symlink, so remove() deletes the directory the OS actually created.

Fixes: #66593
Refs: #64397

AI disclosure: written with the help of Claude (claude:opus-5.5) and Grok (grok-4.7). I checked the bug against the source and the latest nightly, and reviewed the fix and tests.

fs.mkdtempDisposableSync() and fsPromises.mkdtempDisposable() pass the
path of the new directory to path.resolve(), which only accepts
strings. When that path is a Buffer, because the prefix is a Buffer or
`encoding` is 'buffer', both throw ERR_INVALID_ARG_TYPE after the
directory has been created, and nothing removes it.

On Windows, resolve a Buffer path as UTF-8 so win32 semantics stay
intact. On POSIX, keep an absolute Buffer as created and prepend the
cwd captured at creation to a relative one, so bytes that are not
valid UTF-8 are preserved.

Fixes: nodejs#66593
Refs: nodejs#64397
Signed-off-by: Jiawen Geng <technicalcute@gmail.com>
Assisted-by: claude:opus-5.5
Assisted-by: grok-4.7
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run. labels Oct 8, 2026
@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.44%. Comparing base (6d5e309) to head (f88356b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66594      +/-   ##
==========================================
- Coverage   90.44%   90.44%   -0.01%     
==========================================
  Files         791      791              
  Lines      276562   276592      +30     
  Branches    53126    53125       -1     
==========================================
+ Hits       250130   250154      +24     
+ Misses      16846    16841       -5     
- Partials     9586     9597      +11     
Files with missing lines Coverage Δ
lib/fs.js 97.32% <100.00%> (+<0.01%) ⬆️
lib/internal/fs/promises.js 91.15% <100.00%> (+0.13%) ⬆️
lib/internal/fs/utils.js 96.37% <100.00%> (+0.08%) ⬆️

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.mkdtempDisposableSync() / fsPromises.mkdtempDisposable() throw and leak the directory when the path is a Buffer

2 participants