Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rm: shouldn't prompt when stdin is not interactive #7326

Open
jfinkels opened this issue Feb 19, 2025 · 2 comments
Open

rm: shouldn't prompt when stdin is not interactive #7326

jfinkels opened this issue Feb 19, 2025 · 2 comments
Labels

Comments

@jfinkels
Copy link
Collaborator

Environment: Ubuntu 24.04, uutils main branch (git commit a2af5d6), GNU coreutils v9.6.19-7386c-modified

Steps to reproduce:

mkdir -m0 dir
rm -d dir < /dev/null

What happens now: uutils rm prints the interactive prompt but immediately terminates, and leaves the directory as-is:

rm: attempt removal of inaccessible directory 'inacc2'?

What I expected to happen: GNU rm prints nothing and removes the directory silently.

Notes: this is causing a failure in GNU test file tests/rm/empty-inacc.sh. I guess the logic here is that if stdin is not an interactive terminal, then the prompting is disabled? I'm not sure.

@Chrispin-m
Copy link

Chrispin-m commented Feb 20, 2025

The current remove_dir function in in src/uu/rm/src/rm.rs calls prompt_dir to decide whether to proceed with removal. Problem is it does not check if stdin is a terminal, so it attempts to prompt even when stdin is /dev/null, leading to immediate termination without removal. To fix this do:

  1. Add a check using atty::is(Stream::Stdin) to detect if stdin is a terminal.
  2. Skip prompting and proceed with removal when stdin is not a terminal, matching GNU rm’s behavior.
    Here’s the fully rewritten remove_dir function with the fix - (I made the change in here):
/// Remove the given directory, asking the user for permission if necessary.
///
/// Returns true if it has encountered an error.
fn remove_dir(path: &Path, options: &Options) -> bool {
    // Skip prompting if stdin is not a terminal (e.g., redirected from /dev/null)
    if !atty::is(atty::Stream::Stdin) {
        match fs::remove_dir(path) {
            Ok(_) => {
                if options.verbose {
                    println!("removed directory {}", normalize(path).quote());
                }
                return false;
            }
            Err(e) => {
                let e = e.map_err_context(|| format!("cannot remove {}", path.quote()));
                show_error!("{e}");
                return true;
            }
        }
    }
    // Ask the user for permission if stdin is a terminal
    if !prompt_dir(path, options) {
        return false;
    }
    // Called to remove a symlink_dir (windows) without "-r"/"-R" or "-d".
    if !options.dir && !options.recursive {
        show_error!("cannot remove {}: Is a directory", path.quote());
        return true;
    }
    // Try to remove the directory.
    match fs::remove_dir(path) {
        Ok(_) => {
            if options.verbose {
                println!("removed directory {}", normalize(path).quote());
            }
            false
        }
        Err(e) => {
            let e = e.map_err_context(|| format!("cannot remove {}", path.quote()));
            show_error!("{e}");
            true
        }
    }
}

@RenjiSann
Copy link
Collaborator

@Chrispin-m Can you open a PR to contribute your patch ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants