Skip to content

Commit f88356b

Browse files
committed
fs: fix mkdtempDisposable with Buffer paths
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: #66593 Refs: #64397 Signed-off-by: Jiawen Geng <technicalcute@gmail.com> Assisted-by: claude:opus-5.5 Assisted-by: grok-4.7
1 parent 6d5e309 commit f88356b

5 files changed

Lines changed: 220 additions & 2 deletions

File tree

‎lib/fs.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ const {
115115
getStatFsFromBinding,
116116
getStatsFromBinding,
117117
realpathCacheKey,
118+
absoluteMkdtempPath,
118119
stringToFlags,
119120
stringToSymlinkType,
120121
toUnixTimestamp,
@@ -3759,7 +3760,7 @@ function mkdtempDisposableSync(prefix, options) {
37593760

37603761
const path = binding.mkdtemp(prefix, options.encoding);
37613762
// Stash the full path in case of process.chdir()
3762-
const fullPath = pathModule.resolve(process.cwd(), path);
3763+
const fullPath = absoluteMkdtempPath(process.cwd(), path);
37633764

37643765
const remove = () => {
37653766
binding.rmSync(fullPath, 0 /* maxRetries */, true /* recursive */, 100 /* retryDelay */);

‎lib/internal/fs/promises.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ const {
7373
getReadFileBuffer,
7474
getReadFileBufferByteLengthName,
7575
preprocessSymlinkDestination,
76+
absoluteMkdtempPath,
7677
stringToFlags,
7778
stringToSymlinkType,
7879
toUnixTimestamp,
@@ -2109,7 +2110,7 @@ async function mkdtempDisposable(prefix, options) {
21092110
handleErrorFromBinding,
21102111
);
21112112
// Stash the full path in case of process.chdir()
2112-
const fullPath = pathModule.resolve(cwd, path);
2113+
const fullPath = absoluteMkdtempPath(cwd, path);
21132114

21142115
const remove = async () => {
21152116
const rmrf = lazyRimRaf();

‎lib/internal/fs/utils.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@ const {
2525
TypedArrayPrototypeAt,
2626
TypedArrayPrototypeIncludes,
2727
globalThis,
28+
uncurryThis,
2829
} = primordials;
2930

3031
const { Buffer } = require('buffer');
3132
const { isBuffer: BufferIsBuffer } = Buffer;
33+
const BufferToString = uncurryThis(Buffer.prototype.toString);
3234
const {
3335
UVException,
3436
codes: {
@@ -976,6 +978,31 @@ function warnOnNonPortableTemplate(template) {
976978
}
977979
}
978980

981+
/**
982+
* Path passed to rm() for a directory just created by mkdtemp().
983+
* A relative path is made absolute against `cwd` captured at creation, so
984+
* remove() still finds the directory after process.chdir().
985+
* @param {string} cwd
986+
* @param {string | Buffer} path
987+
* @returns {string | Buffer}
988+
*/
989+
function absoluteMkdtempPath(cwd, path) {
990+
if (typeof path === 'string') {
991+
return pathModule.resolve(cwd, path);
992+
}
993+
// Windows Buffer paths are UTF-8. Resolve the string so root-relative
994+
// (`\foo`) and drive-relative (`D:foo`) prefixes keep win32 semantics.
995+
if (isWindows) {
996+
return Buffer.from(pathModule.resolve(cwd, BufferToString(path, 'utf8')));
997+
}
998+
// POSIX names are bytes and may not be valid UTF-8. Keep an absolute
999+
// result as it was created. Prepend cwd to a relative one.
1000+
if (path.length > 0 && path[0] === CHAR_FORWARD_SLASH) {
1001+
return Buffer.from(path);
1002+
}
1003+
return join(cwd, path);
1004+
}
1005+
9791006
const defaultCpOptions = {
9801007
dereference: false,
9811008
errorOnExist: false,
@@ -1216,6 +1243,7 @@ module.exports = {
12161243
handleErrorFromBinding,
12171244
preprocessSymlinkDestination,
12181245
realpathCacheKey: Symbol('realpathCacheKey'),
1246+
absoluteMkdtempPath,
12191247
getStatFsFromBinding,
12201248
getStatsFromBinding,
12211249
stringToFlags,

‎test/parallel/test-fs-mkdtempDisposableSync.js‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,97 @@ if (isMainThread) {
6666
process.chdir(originalCwd);
6767
}
6868

69+
// Buffer paths
70+
{
71+
const prefix = tmpdir.resolve('foo.');
72+
for (const result of [
73+
fs.mkdtempDisposableSync(Buffer.from(prefix)),
74+
fs.mkdtempDisposableSync(prefix, { encoding: 'buffer' }),
75+
]) {
76+
assert(Buffer.isBuffer(result.path));
77+
assert.strictEqual(path.dirname(result.path.toString()), tmpdir.path);
78+
assert(fs.existsSync(result.path));
79+
80+
result.remove();
81+
82+
assert(!fs.existsSync(result.path));
83+
}
84+
}
85+
86+
// `chdir` does not affect removal of a relative Buffer path
87+
// Can't use chdir in workers
88+
if (isMainThread) {
89+
const originalCwd = process.cwd();
90+
91+
process.chdir(tmpdir.path);
92+
const result = fs.mkdtempDisposableSync(Buffer.from('buffer.'));
93+
const fullPath = path.join(tmpdir.path, result.path.toString());
94+
95+
assert(fs.existsSync(fullPath));
96+
97+
process.chdir(originalCwd);
98+
result.remove();
99+
100+
assert(!fs.existsSync(fullPath));
101+
}
102+
103+
// Buffer paths that are not valid UTF-8 are removed as is
104+
// macOS rejects such file names, and Windows converts paths from UTF-8
105+
if (common.isLinux) {
106+
const prefix = Buffer.concat([Buffer.from(tmpdir.resolve('foo')), Buffer.from([0xff, 0x2e])]);
107+
const result = fs.mkdtempDisposableSync(prefix);
108+
109+
assert.deepStrictEqual(result.path.subarray(0, prefix.length), prefix);
110+
assert(fs.existsSync(result.path));
111+
112+
result.remove();
113+
114+
assert(!fs.existsSync(result.path));
115+
}
116+
117+
// Relative Buffer path under a non-ASCII cwd
118+
// Can't use chdir in workers
119+
if (isMainThread) {
120+
const originalCwd = process.cwd();
121+
const nonAscii = fs.mkdtempSync(path.join(tmpdir.path, '\u7528\u6237-'));
122+
123+
process.chdir(nonAscii);
124+
const result = fs.mkdtempDisposableSync(Buffer.from('buffer.'));
125+
const fullPath = path.join(nonAscii, result.path.toString());
126+
127+
assert(fs.existsSync(fullPath));
128+
129+
process.chdir(originalCwd);
130+
result.remove();
131+
132+
assert(!fs.existsSync(fullPath));
133+
fs.rmSync(nonAscii, { recursive: true });
134+
}
135+
136+
// `..` after a symlink is resolved by the OS at creation. remove() has to
137+
// use those bytes; lexical normalization points at a directory that was
138+
// never created. Windows normalizes `..` before following the symlink.
139+
if (!common.isWindows) {
140+
const outside = fs.mkdtempSync(path.join(tmpdir.path, 'outside-'));
141+
const parent = fs.mkdtempSync(path.join(tmpdir.path, 'parent-'));
142+
const link = path.join(parent, 'link');
143+
fs.symlinkSync(outside, link);
144+
145+
const prefix = Buffer.from(`${link}/../foo.`);
146+
const result = fs.mkdtempDisposableSync(prefix);
147+
const baseName = path.basename(result.path.toString());
148+
const createdPath = path.join(path.dirname(outside), baseName);
149+
150+
assert(fs.existsSync(createdPath));
151+
assert(!fs.existsSync(path.join(parent, baseName)));
152+
153+
result.remove();
154+
155+
assert(!fs.existsSync(createdPath));
156+
fs.rmSync(outside, { recursive: true });
157+
fs.rmSync(parent, { recursive: true });
158+
}
159+
69160
// Errors from cleanup are thrown
70161
// It is difficult to arrange for rmdir to fail on windows
71162
if (!common.isWindows && process.getuid() !== 0) {

‎test/parallel/test-fs-promises-mkdtempDisposable.js‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,98 @@ async function chdirDoesNotAffectRemoval() {
6666
process.chdir(originalCwd);
6767
}
6868

69+
async function bufferPaths() {
70+
const prefix = tmpdir.resolve('foo.');
71+
for (const result of [
72+
await fsPromises.mkdtempDisposable(Buffer.from(prefix)),
73+
await fsPromises.mkdtempDisposable(prefix, { encoding: 'buffer' }),
74+
]) {
75+
assert(Buffer.isBuffer(result.path));
76+
assert.strictEqual(path.dirname(result.path.toString()), tmpdir.path);
77+
assert(fs.existsSync(result.path));
78+
79+
await result.remove();
80+
81+
assert(!fs.existsSync(result.path));
82+
}
83+
}
84+
85+
async function chdirDoesNotAffectRemovalOfBufferPath() {
86+
// Can't use chdir in workers
87+
if (!isMainThread) return;
88+
89+
const originalCwd = process.cwd();
90+
91+
process.chdir(tmpdir.path);
92+
const result = await fsPromises.mkdtempDisposable(Buffer.from('buffer.'));
93+
const fullPath = path.join(tmpdir.path, result.path.toString());
94+
95+
assert(fs.existsSync(fullPath));
96+
97+
process.chdir(originalCwd);
98+
await result.remove();
99+
100+
assert(!fs.existsSync(fullPath));
101+
}
102+
103+
async function nonUtf8BufferPath() {
104+
// macOS rejects such file names, and Windows converts paths from UTF-8
105+
if (!common.isLinux) return;
106+
const prefix = Buffer.concat([Buffer.from(tmpdir.resolve('foo')), Buffer.from([0xff, 0x2e])]);
107+
const result = await fsPromises.mkdtempDisposable(prefix);
108+
109+
assert.deepStrictEqual(result.path.subarray(0, prefix.length), prefix);
110+
assert(fs.existsSync(result.path));
111+
112+
await result.remove();
113+
114+
assert(!fs.existsSync(result.path));
115+
}
116+
117+
async function relativeBufferPathInNonAsciiCwd() {
118+
// Can't use chdir in workers
119+
if (!isMainThread) return;
120+
121+
const originalCwd = process.cwd();
122+
const nonAscii = fs.mkdtempSync(path.join(tmpdir.path, '\u7528\u6237-'));
123+
124+
process.chdir(nonAscii);
125+
const result = await fsPromises.mkdtempDisposable(Buffer.from('buffer.'));
126+
const fullPath = path.join(nonAscii, result.path.toString());
127+
128+
assert(fs.existsSync(fullPath));
129+
130+
process.chdir(originalCwd);
131+
await result.remove();
132+
133+
assert(!fs.existsSync(fullPath));
134+
fs.rmSync(nonAscii, { recursive: true });
135+
}
136+
137+
async function symlinkDotDotKeepsCreatedDirectory() {
138+
// Windows normalizes `..` before following the symlink.
139+
if (common.isWindows) return;
140+
141+
const outside = fs.mkdtempSync(path.join(tmpdir.path, 'outside-'));
142+
const parent = fs.mkdtempSync(path.join(tmpdir.path, 'parent-'));
143+
const link = path.join(parent, 'link');
144+
fs.symlinkSync(outside, link);
145+
146+
const prefix = Buffer.from(`${link}/../foo.`);
147+
const result = await fsPromises.mkdtempDisposable(prefix);
148+
const baseName = path.basename(result.path.toString());
149+
const createdPath = path.join(path.dirname(outside), baseName);
150+
151+
assert(fs.existsSync(createdPath));
152+
assert(!fs.existsSync(path.join(parent, baseName)));
153+
154+
await result.remove();
155+
156+
assert(!fs.existsSync(createdPath));
157+
fs.rmSync(outside, { recursive: true });
158+
fs.rmSync(parent, { recursive: true });
159+
}
160+
69161
async function errorsAreReThrown() {
70162
// It is difficult to arrange for rmdir to fail on windows
71163
if (common.isWindows || process.getuid() === 0) return;
@@ -93,5 +185,10 @@ async function errorsAreReThrown() {
93185
await basicUsage();
94186
await symbolAsyncDispose();
95187
await chdirDoesNotAffectRemoval();
188+
await bufferPaths();
189+
await chdirDoesNotAffectRemovalOfBufferPath();
190+
await nonUtf8BufferPath();
191+
await relativeBufferPathInNonAsciiCwd();
192+
await symlinkDotDotKeepsCreatedDirectory();
96193
await errorsAreReThrown();
97194
})().then(common.mustCall());

0 commit comments

Comments
 (0)