From 4b11b283ac64265a52834164d26d4b5df002e32c Mon Sep 17 00:00:00 2001
From: Lily Mara <6109875+lily-mara@users.noreply.github.com>
Date: Sat, 29 May 2021 20:36:36 -0700
Subject: [PATCH] Resolve issues with `rm *` globbing (#3516)

Using the `*` wildcard should not attempt to delete files with a leading dot
unless the more explicit `.*` is used. `rm *` should also not attempt to delete
the current directory or its parent directory (`.` and `..`). I have resolved
this bug as well in a less satisfactory way. I think it may be the case that we
can only disambiguate the `.` and `..` path segments by using `Path::display`.
Here is a short list of alternatives that I tried:

- `Path::ends_with()` can detect `/..` but not `/.`.
- `Path::iter()` and `Path::components()` leave out `/.`.
- `Path::file_name()` normalizes `/.` to the parent component's file name.

Fixes #3508
---
 crates/nu-command/tests/commands/rm.rs        | 30 +++++++++++++++++++
 .../src/filesystem/filesystem_shell.rs        | 16 +++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/crates/nu-command/tests/commands/rm.rs b/crates/nu-command/tests/commands/rm.rs
index f43d8fb607..4a20eb3821 100644
--- a/crates/nu-command/tests/commands/rm.rs
+++ b/crates/nu-command/tests/commands/rm.rs
@@ -276,3 +276,33 @@ fn no_errors_if_attempting_to_delete_non_existent_file_with_f_flag() {
         assert!(!actual.err.contains("no valid path"));
     })
 }
+
+#[test]
+fn rm_wildcard_keeps_dotfiles() {
+    Playground::setup("rm_test_15", |dirs, sandbox| {
+        sandbox.with_files(vec![EmptyFile("foo"), EmptyFile(".bar")]);
+
+        nu!(
+            cwd: dirs.test(),
+            r#"rm *"#
+        );
+
+        assert!(!files_exist_at(vec!["foo"], dirs.test()));
+        assert!(files_exist_at(vec![".bar"], dirs.test()));
+    })
+}
+
+#[test]
+fn rm_wildcard_leading_dot_deletes_dotfiles() {
+    Playground::setup("rm_test_16", |dirs, sandbox| {
+        sandbox.with_files(vec![EmptyFile("foo"), EmptyFile(".bar")]);
+
+        nu!(
+            cwd: dirs.test(),
+            r#"rm .*"#
+        );
+
+        assert!(files_exist_at(vec!["foo"], dirs.test()));
+        assert!(!files_exist_at(vec![".bar"], dirs.test()));
+    })
+}
diff --git a/crates/nu-engine/src/filesystem/filesystem_shell.rs b/crates/nu-engine/src/filesystem/filesystem_shell.rs
index 2a531aa3a0..75cdb027cb 100644
--- a/crates/nu-engine/src/filesystem/filesystem_shell.rs
+++ b/crates/nu-engine/src/filesystem/filesystem_shell.rs
@@ -643,11 +643,25 @@ impl Shell for FilesystemShell {
             }
 
             let path = path.join(&target.item);
-            match glob::glob(&path.to_string_lossy()) {
+            match glob::glob_with(
+                &path.to_string_lossy(),
+                glob::MatchOptions {
+                    require_literal_leading_dot: true,
+                    ..Default::default()
+                },
+            ) {
                 Ok(files) => {
                     for file in files {
                         match file {
                             Ok(ref f) => {
+                                // It is not appropriate to try and remove the
+                                // current directory or its parent when using
+                                // glob patterns.
+                                let name = format!("{}", f.display());
+                                if name.ends_with("/.") || name.ends_with("/..") {
+                                    continue;
+                                }
+
                                 all_targets
                                     .entry(f.clone())
                                     .or_insert_with(|| target.tag.clone());