Skip to content

Commit fc17efe

Browse files
GomesGoncalosylvestre
authored andcommitted
rm: don't treat symlinks as write-protected
GNU rm does not check for write-protection on symbolic links; it instead prompts to "remove symbolic link" regardless of the link's permissions or its target's status. This change: - Ensures `prompt_file` checks for symlinks specifically using `symlink_metadata`, avoiding the incorrect "write-protected" prompt. - Refactors permission checks into `is_writable_metadata` to allow using the already-fetched metadata, which also optimizes performance by reducing redundant `stat` calls. - Updates `prompt_file_permission_readonly` to operate on metadata directly.
1 parent f4ed162 commit fc17efe

2 files changed

Lines changed: 32 additions & 28 deletions

File tree

src/uu/rm/src/rm.rs

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -549,19 +549,8 @@ fn is_writable_metadata(metadata: &Metadata) -> bool {
549549
(mode & 0o200) > 0
550550
}
551551

552-
/// Whether the given file or directory is writable.
553-
#[cfg(unix)]
554-
fn is_writable(path: &Path) -> bool {
555-
match fs::metadata(path) {
556-
Err(_) => false,
557-
Ok(metadata) => is_writable_metadata(&metadata),
558-
}
559-
}
560-
561-
/// Whether the given file or directory is writable.
562552
#[cfg(not(unix))]
563-
fn is_writable(_path: &Path) -> bool {
564-
// TODO Not yet implemented.
553+
fn is_writable_metadata(_metadata: &Metadata) -> bool {
565554
true
566555
}
567556

@@ -799,35 +788,33 @@ fn prompt_file(path: &Path, options: &Options) -> bool {
799788
if options.interactive == InteractiveMode::Never {
800789
return true;
801790
}
802-
// If interactive is Always we want to check if the file is symlink to prompt the right message
803-
if options.interactive == InteractiveMode::Always {
804-
if let Ok(metadata) = fs::symlink_metadata(path) {
805-
if metadata.is_symlink() {
806-
return prompt_yes!("remove symbolic link {}?", path.quote());
807-
}
808-
}
809-
}
810791

811-
let Ok(metadata) = fs::metadata(path) else {
792+
let Ok(metadata) = fs::symlink_metadata(path) else {
812793
return true;
813794
};
814795

815-
if options.interactive == InteractiveMode::Always && is_writable(path) {
796+
if metadata.is_symlink() {
797+
return options.interactive != InteractiveMode::Always
798+
|| prompt_yes!("remove symbolic link {}?", path.quote());
799+
}
800+
801+
if options.interactive == InteractiveMode::Always && is_writable_metadata(&metadata) {
816802
return if metadata.len() == 0 {
817803
prompt_yes!("remove regular empty file {}?", path.quote())
818804
} else {
819805
prompt_yes!("remove file {}?", path.quote())
820806
};
821807
}
822-
prompt_file_permission_readonly(path, options)
808+
809+
prompt_file_permission_readonly(path, options, &metadata)
823810
}
824811

825-
fn prompt_file_permission_readonly(path: &Path, options: &Options) -> bool {
812+
fn prompt_file_permission_readonly(path: &Path, options: &Options, metadata: &Metadata) -> bool {
826813
let stdin_ok = options.__presume_input_tty.unwrap_or(false) || stdin().is_terminal();
827-
match (stdin_ok, fs::metadata(path), options.interactive) {
828-
(false, _, InteractiveMode::PromptProtected) => true,
829-
(_, Ok(_), _) if is_writable(path) => true,
830-
(_, Ok(metadata), _) if metadata.len() == 0 => prompt_yes!(
814+
match (stdin_ok, options.interactive) {
815+
(false, InteractiveMode::PromptProtected) => true,
816+
_ if is_writable_metadata(metadata) => true,
817+
_ if metadata.len() == 0 => prompt_yes!(
831818
"remove write-protected regular empty file {}?",
832819
path.quote()
833820
),

tests/by-util/test_rm.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,3 +1217,20 @@ fn test_progress_no_output_on_error() {
12171217
.stderr_contains("cannot remove")
12181218
.stderr_contains("No such file or directory");
12191219
}
1220+
1221+
#[cfg(unix)]
1222+
#[test]
1223+
fn test_symlink_to_readonly_no_prompt() {
1224+
let (at, mut ucmd) = at_and_ucmd!();
1225+
1226+
at.touch("foo");
1227+
at.set_mode("foo", 0o444);
1228+
at.symlink_file("foo", "bar");
1229+
1230+
ucmd.arg("---presume-input-tty")
1231+
.arg("bar")
1232+
.succeeds()
1233+
.no_stderr();
1234+
1235+
assert!(!at.symlink_exists("bar"));
1236+
}

0 commit comments

Comments
 (0)