Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b305ad4

Browse filesBrowse files
nathanael-rufdanielleadams
nathanael-ruf
authored and
danielleadams
committed
fs: fix fs.rm support for loop symlinks
Fixes: #45404 PR-URL: #45439 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
1 parent 6f8759b commit b305ad4
Copy full SHA for b305ad4

File tree

Expand file treeCollapse file tree

2 files changed

+136
-7
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+136
-7
lines changed
Open diff view settings
Collapse file

‎lib/internal/fs/utils.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/utils.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ const validateRmOptions = hideStackFrames((path, options, expectDir, cb) => {
770770
options = validateRmdirOptions(options, defaultRmOptions);
771771
validateBoolean(options.force, 'options.force');
772772

773-
lazyLoadFs().stat(path, (err, stats) => {
773+
lazyLoadFs().lstat(path, (err, stats) => {
774774
if (err) {
775775
if (options.force && err.code === 'ENOENT') {
776776
return cb(null, options);
@@ -801,7 +801,7 @@ const validateRmOptionsSync = hideStackFrames((path, options, expectDir) => {
801801

802802
if (!options.force || expectDir || !options.recursive) {
803803
const isDirectory = lazyLoadFs()
804-
.statSync(path, { throwIfNoEntry: !options.force })?.isDirectory();
804+
.lstatSync(path, { throwIfNoEntry: !options.force })?.isDirectory();
805805

806806
if (expectDir && !isDirectory) {
807807
return false;
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-fs-rm.js
+134-5Lines changed: 134 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,15 @@ function makeNonEmptyDirectory(depth, files, folders, dirname, createSymLinks) {
4949
path.join(dirname, `link-${depth}-bad`),
5050
'file'
5151
);
52+
53+
// Symlinks that form a loop
54+
[['a', 'b'], ['b', 'a']].forEach(([x, y]) => {
55+
fs.symlinkSync(
56+
`link-${depth}-loop-${x}`,
57+
path.join(dirname, `link-${depth}-loop-${y}`),
58+
'file'
59+
);
60+
});
5261
}
5362

5463
// File with a name that looks like a glob
@@ -88,7 +97,7 @@ function removeAsync(dir) {
8897

8998
// Attempted removal should fail now because the directory is gone.
9099
fs.rm(dir, common.mustCall((err) => {
91-
assert.strictEqual(err.syscall, 'stat');
100+
assert.strictEqual(err.syscall, 'lstat');
92101
}));
93102
}));
94103
}));
@@ -137,6 +146,48 @@ function removeAsync(dir) {
137146
fs.rmSync(filePath, common.mustNotMutateObjectDeep({ force: true }));
138147
}
139148
}));
149+
150+
// Should delete a valid symlink
151+
const linkTarget = path.join(tmpdir.path, 'link-target-async.txt');
152+
fs.writeFileSync(linkTarget, '');
153+
const validLink = path.join(tmpdir.path, 'valid-link-async');
154+
fs.symlinkSync(linkTarget, validLink);
155+
fs.rm(validLink, common.mustNotMutateObjectDeep({ recursive: true }), common.mustCall((err) => {
156+
try {
157+
assert.strictEqual(err, null);
158+
assert.strictEqual(fs.existsSync(validLink), false);
159+
} finally {
160+
fs.rmSync(linkTarget, common.mustNotMutateObjectDeep({ force: true }));
161+
fs.rmSync(validLink, common.mustNotMutateObjectDeep({ force: true }));
162+
}
163+
}));
164+
165+
// Should delete an invalid symlink
166+
const invalidLink = path.join(tmpdir.path, 'invalid-link-async');
167+
fs.symlinkSync('definitely-does-not-exist-async', invalidLink);
168+
fs.rm(invalidLink, common.mustNotMutateObjectDeep({ recursive: true }), common.mustCall((err) => {
169+
try {
170+
assert.strictEqual(err, null);
171+
assert.strictEqual(fs.existsSync(invalidLink), false);
172+
} finally {
173+
fs.rmSync(invalidLink, common.mustNotMutateObjectDeep({ force: true }));
174+
}
175+
}));
176+
177+
// Should delete a symlink that is part of a loop
178+
const loopLinkA = path.join(tmpdir.path, 'loop-link-async-a');
179+
const loopLinkB = path.join(tmpdir.path, 'loop-link-async-b');
180+
fs.symlinkSync(loopLinkA, loopLinkB);
181+
fs.symlinkSync(loopLinkB, loopLinkA);
182+
fs.rm(loopLinkA, common.mustNotMutateObjectDeep({ recursive: true }), common.mustCall((err) => {
183+
try {
184+
assert.strictEqual(err, null);
185+
assert.strictEqual(fs.existsSync(loopLinkA), false);
186+
} finally {
187+
fs.rmSync(loopLinkA, common.mustNotMutateObjectDeep({ force: true }));
188+
fs.rmSync(loopLinkB, common.mustNotMutateObjectDeep({ force: true }));
189+
}
190+
}));
140191
}
141192

142193
// Removing a .git directory should not throw an EPERM.
@@ -168,7 +219,7 @@ if (isGitPresent) {
168219
}, {
169220
code: 'ENOENT',
170221
name: 'Error',
171-
message: /^ENOENT: no such file or directory, stat/
222+
message: /^ENOENT: no such file or directory, lstat/
172223
});
173224

174225
// Should delete a file
@@ -177,25 +228,64 @@ if (isGitPresent) {
177228

178229
try {
179230
fs.rmSync(filePath, common.mustNotMutateObjectDeep({ recursive: true }));
231+
assert.strictEqual(fs.existsSync(filePath), false);
180232
} finally {
181233
fs.rmSync(filePath, common.mustNotMutateObjectDeep({ force: true }));
182234
}
183235

236+
// Should delete a valid symlink
237+
const linkTarget = path.join(tmpdir.path, 'link-target.txt');
238+
fs.writeFileSync(linkTarget, '');
239+
const validLink = path.join(tmpdir.path, 'valid-link');
240+
fs.symlinkSync(linkTarget, validLink);
241+
try {
242+
fs.rmSync(validLink);
243+
assert.strictEqual(fs.existsSync(validLink), false);
244+
} finally {
245+
fs.rmSync(linkTarget, common.mustNotMutateObjectDeep({ force: true }));
246+
fs.rmSync(validLink, common.mustNotMutateObjectDeep({ force: true }));
247+
}
248+
249+
// Should delete an invalid symlink
250+
const invalidLink = path.join(tmpdir.path, 'invalid-link');
251+
fs.symlinkSync('definitely-does-not-exist', invalidLink);
252+
try {
253+
fs.rmSync(invalidLink);
254+
assert.strictEqual(fs.existsSync(invalidLink), false);
255+
} finally {
256+
fs.rmSync(invalidLink, common.mustNotMutateObjectDeep({ force: true }));
257+
}
258+
259+
// Should delete a symlink that is part of a loop
260+
const loopLinkA = path.join(tmpdir.path, 'loop-link-a');
261+
const loopLinkB = path.join(tmpdir.path, 'loop-link-b');
262+
fs.symlinkSync(loopLinkA, loopLinkB);
263+
fs.symlinkSync(loopLinkB, loopLinkA);
264+
try {
265+
fs.rmSync(loopLinkA);
266+
assert.strictEqual(fs.existsSync(loopLinkA), false);
267+
} finally {
268+
fs.rmSync(loopLinkA, common.mustNotMutateObjectDeep({ force: true }));
269+
fs.rmSync(loopLinkB, common.mustNotMutateObjectDeep({ force: true }));
270+
}
271+
184272
// Should accept URL
185273
const fileURL = pathToFileURL(path.join(tmpdir.path, 'rm-file.txt'));
186274
fs.writeFileSync(fileURL, '');
187275

188276
try {
189277
fs.rmSync(fileURL, common.mustNotMutateObjectDeep({ recursive: true }));
278+
assert.strictEqual(fs.existsSync(fileURL), false);
190279
} finally {
191280
fs.rmSync(fileURL, common.mustNotMutateObjectDeep({ force: true }));
192281
}
193282

194283
// Recursive removal should succeed.
195284
fs.rmSync(dir, { recursive: true });
285+
assert.strictEqual(fs.existsSync(dir), false);
196286

197287
// Attempted removal should fail now because the directory is gone.
198-
assert.throws(() => fs.rmSync(dir), { syscall: 'stat' });
288+
assert.throws(() => fs.rmSync(dir), { syscall: 'lstat' });
199289
}
200290

201291
// Removing a .git directory should not throw an EPERM.
@@ -220,9 +310,10 @@ if (isGitPresent) {
220310

221311
// Recursive removal should succeed.
222312
await fs.promises.rm(dir, common.mustNotMutateObjectDeep({ recursive: true }));
313+
assert.strictEqual(fs.existsSync(dir), false);
223314

224315
// Attempted removal should fail now because the directory is gone.
225-
await assert.rejects(fs.promises.rm(dir), { syscall: 'stat' });
316+
await assert.rejects(fs.promises.rm(dir), { syscall: 'lstat' });
226317

227318
// Should fail if target does not exist
228319
await assert.rejects(fs.promises.rm(
@@ -231,7 +322,7 @@ if (isGitPresent) {
231322
), {
232323
code: 'ENOENT',
233324
name: 'Error',
234-
message: /^ENOENT: no such file or directory, stat/
325+
message: /^ENOENT: no such file or directory, lstat/
235326
});
236327

237328
// Should not fail if target does not exist and force option is true
@@ -243,16 +334,54 @@ if (isGitPresent) {
243334

244335
try {
245336
await fs.promises.rm(filePath, common.mustNotMutateObjectDeep({ recursive: true }));
337+
assert.strictEqual(fs.existsSync(filePath), false);
246338
} finally {
247339
fs.rmSync(filePath, common.mustNotMutateObjectDeep({ force: true }));
248340
}
249341

342+
// Should delete a valid symlink
343+
const linkTarget = path.join(tmpdir.path, 'link-target-prom.txt');
344+
fs.writeFileSync(linkTarget, '');
345+
const validLink = path.join(tmpdir.path, 'valid-link-prom');
346+
fs.symlinkSync(linkTarget, validLink);
347+
try {
348+
await fs.promises.rm(validLink);
349+
assert.strictEqual(fs.existsSync(validLink), false);
350+
} finally {
351+
fs.rmSync(linkTarget, common.mustNotMutateObjectDeep({ force: true }));
352+
fs.rmSync(validLink, common.mustNotMutateObjectDeep({ force: true }));
353+
}
354+
355+
// Should delete an invalid symlink
356+
const invalidLink = path.join(tmpdir.path, 'invalid-link-prom');
357+
fs.symlinkSync('definitely-does-not-exist-prom', invalidLink);
358+
try {
359+
await fs.promises.rm(invalidLink);
360+
assert.strictEqual(fs.existsSync(invalidLink), false);
361+
} finally {
362+
fs.rmSync(invalidLink, common.mustNotMutateObjectDeep({ force: true }));
363+
}
364+
365+
// Should delete a symlink that is part of a loop
366+
const loopLinkA = path.join(tmpdir.path, 'loop-link-prom-a');
367+
const loopLinkB = path.join(tmpdir.path, 'loop-link-prom-b');
368+
fs.symlinkSync(loopLinkA, loopLinkB);
369+
fs.symlinkSync(loopLinkB, loopLinkA);
370+
try {
371+
await fs.promises.rm(loopLinkA);
372+
assert.strictEqual(fs.existsSync(loopLinkA), false);
373+
} finally {
374+
fs.rmSync(loopLinkA, common.mustNotMutateObjectDeep({ force: true }));
375+
fs.rmSync(loopLinkB, common.mustNotMutateObjectDeep({ force: true }));
376+
}
377+
250378
// Should accept URL
251379
const fileURL = pathToFileURL(path.join(tmpdir.path, 'rm-promises-file.txt'));
252380
fs.writeFileSync(fileURL, '');
253381

254382
try {
255383
await fs.promises.rm(fileURL, common.mustNotMutateObjectDeep({ recursive: true }));
384+
assert.strictEqual(fs.existsSync(fileURL), false);
256385
} finally {
257386
fs.rmSync(fileURL, common.mustNotMutateObjectDeep({ force: true }));
258387
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.