This commit is contained in:
Zack Radisic
2025-07-21 20:48:37 -07:00
parent 5fbd99e0cb
commit 9e028be831
4 changed files with 50 additions and 5 deletions

View File

@@ -923,7 +923,7 @@ pub const ShellRmTask = struct {
}
debug("[removeEntryDir] remove after children {s}", .{path});
switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR)) {
switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR | std.posix.AT.SYMLINK_NOFOLLOW)) {
.result => {
switch (this.verboseDeleted(dir_task, path)) {
.err => |e| return .{ .err = e },
@@ -1083,7 +1083,7 @@ pub const ShellRmTask = struct {
}
};
const dirfd = this.cwd;
switch (ShellSyscall.unlinkatWithFlags(dirfd, path, 0)) {
switch (ShellSyscall.unlinkatWithFlags(dirfd, path, std.posix.AT.SYMLINK_NOFOLLOW)) {
.result => return this.verboseDeleted(parent_dir_task, path),
.err => |e| {
debug("unlinkatWithFlags({s}) = {s}", .{ path, @tagName(e.getErrno()) });
@@ -1107,7 +1107,7 @@ pub const ShellRmTask = struct {
// If `path` points to a directory, then it is deleted (if empty) or we handle it as a directory
// If it's actually a file, we get an error so we don't need to call `stat` to check that.
if (this.opts.recursive or this.opts.remove_empty_dirs) {
return switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR)) {
return switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR | std.posix.AT.SYMLINK_NOFOLLOW)) {
// it was empty, we saved a syscall
.result => return this.verboseDeleted(parent_dir_task, path),
.err => |e2| {

View File

@@ -2834,9 +2834,11 @@ pub fn unlinkatWithFlags(dirfd: bun.FileDescriptor, to: anytype, flags: c_uint)
return unlinkatWithFlags(dirfd, bun.strings.toNTPath(w_buf, bun.span(to)), flags);
}
// const follow_symlinks = flags
return bun.windows.DeleteFileBun(to, .{
.dir = if (dirfd != bun.invalid_fd) dirfd.cast() else null,
.remove_dir = flags & std.posix.AT.REMOVEDIR != 0,
.follow_symlinks = flags & std.posix.AT.SYMLINK_NOFOLLOW == 0,
});
}

View File

@@ -3483,6 +3483,7 @@ pub extern "kernel32" fn SetConsoleCP(wCodePageID: std.os.windows.UINT) callconv
pub const DeleteFileOptions = struct {
dir: ?HANDLE,
remove_dir: bool = false,
follow_symlinks: bool = true,
};
const FILE_DISPOSITION_DELETE: ULONG = 0x00000001;
@@ -3493,10 +3494,11 @@ const FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE: ULONG = 0x00000010;
// Copy-paste of the standard library function except without unreachable.
pub fn DeleteFileBun(sub_path_w: []const u16, options: DeleteFileOptions) bun.JSC.Maybe(void) {
const FOLLOW_SYMLINKS = if (options.follow_symlinks) FILE_OPEN_REPARSE_POINT else 0;
const create_options_flags: ULONG = if (options.remove_dir)
FILE_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT
FILE_DIRECTORY_FILE | FOLLOW_SYMLINKS
else
windows.FILE_NON_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT; // would we ever want to delete the target instead?
windows.FILE_NON_DIRECTORY_FILE | FOLLOW_SYMLINKS; // would we ever want to delete the target instead? Yes, `rm` is expected to delete symlinks, not the actual files
const path_len_bytes = @as(u16, @intCast(sub_path_w.len * 2));
var nt_name = UNICODE_STRING{

View File

@@ -144,6 +144,47 @@ foo/
expect(await fileExists(`${tempdir}/sub_dir_files`)).toBeTrue();
}
});
test("removes symlinks, not the files referenced by the links", async () => {
const tempdir = tempDirWithFiles("rm-symlinks", {
"target.txt": "original content",
"dir/file.txt": "directory file",
});
// Create symlinks
await $`ln -s ${tempdir}/target.txt ${tempdir}/link.txt`.cwd(tempdir);
await $`ln -s ${tempdir}/dir ${tempdir}/dirlink`.cwd(tempdir);
// Verify symlinks exist and point to correct targets
expect(await fileExists(`${tempdir}/link.txt`)).toBeTrue();
expect(await fileExists(`${tempdir}/dirlink`)).toBeTrue();
expect(await Bun.file(`${tempdir}/link.txt`).text()).toBe("original content");
expect(await Bun.file(`${tempdir}/dirlink/file.txt`).text()).toBe("directory file");
// Remove the symlinks
{
const { stdout, exitCode } = await $`rm -v ${tempdir}/link.txt`;
expect(stdout.toString()).toEqual(`${tempdir}/link.txt\n`);
expect(exitCode).toBe(0);
}
{
const { stdout, exitCode } = await $`rm -v ${tempdir}/dirlink`;
expect(stdout.toString()).toEqual(`${tempdir}/dirlink\n`);
expect(exitCode).toBe(0);
}
// Verify symlinks are gone but targets remain
expect(await fileExists(`${tempdir}/link.txt`)).toBeFalse();
expect(await fileExists(`${tempdir}/dirlink`)).toBeFalse();
expect(await fileExists(`${tempdir}/target.txt`)).toBeTrue();
expect(await fileExists(`${tempdir}/dir`)).toBeTrue();
expect(await fileExists(`${tempdir}/dir/file.txt`)).toBeTrue();
// Verify target files still have their content
expect(await Bun.file(`${tempdir}/target.txt`).text()).toBe("original content");
expect(await Bun.file(`${tempdir}/dir/file.txt`).text()).toBe("directory file");
});
});
function packagejson() {