From 6ae78847860f05e57a676f807535e9e21ea505b7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= <kubouch@gmail.com>
Date: Wed, 26 May 2021 11:17:18 +0300
Subject: [PATCH] Fix path dots expansion (#3491)

* Fix parser expanding dots where it shouldn't

Previously, the parser would expand "a...b" as "a../..b". Now, >2 dots
are only expanded when the whole path component consists of dots (i.e.,
"..." expands to "../.." while "a...b" stays as it is).

* Respect OS separator when expanding >2 dots

"..." now expands to either "../.." or "..\..", based on the host OS.
---
 crates/nu-parser/src/path.rs | 103 ++++++++++++++++++++++++++++++++---
 1 file changed, 94 insertions(+), 9 deletions(-)

diff --git a/crates/nu-parser/src/path.rs b/crates/nu-parser/src/path.rs
index 7c018d9fb..379886a89 100644
--- a/crates/nu-parser/src/path.rs
+++ b/crates/nu-parser/src/path.rs
@@ -1,5 +1,7 @@
 use std::borrow::Cow;
 
+const EXPAND_STR: &str = if cfg!(windows) { r"..\" } else { "../" };
+
 fn handle_dots_push(string: &mut String, count: u8) {
     if count < 1 {
         return;
@@ -11,20 +13,34 @@ fn handle_dots_push(string: &mut String, count: u8) {
     }
 
     for _ in 0..(count - 1) {
-        string.push_str("../");
+        string.push_str(EXPAND_STR);
     }
 
     string.pop(); // remove last '/'
 }
 
 pub fn expand_ndots(path: &str) -> Cow<'_, str> {
+    // helpers
+    #[cfg(windows)]
+    fn is_separator(c: char) -> bool {
+        // AFAIK, Windows can have both \ and / as path components separators
+        (c == '/') || (c == '\\')
+    }
+
+    #[cfg(not(windows))]
+    fn is_separator(c: char) -> bool {
+        c == '/'
+    }
+
+    // find if we need to expand any >2 dot paths and early exit if not
     let mut dots_count = 0u8;
     let ndots_present = {
         for chr in path.chars() {
             if chr == '.' {
                 dots_count += 1;
             } else {
-                if dots_count > 2 {
+                if is_separator(chr) && (dots_count > 2) {
+                    // this path component had >2 dots
                     break;
                 }
 
@@ -42,12 +58,21 @@ pub fn expand_ndots(path: &str) -> Cow<'_, str> {
     let mut dots_count = 0u8;
     let mut expanded = String::new();
     for chr in path.chars() {
-        if chr != '.' {
-            handle_dots_push(&mut expanded, dots_count);
-            dots_count = 0;
-            expanded.push(chr);
-        } else {
+        if chr == '.' {
             dots_count += 1;
+        } else {
+            if is_separator(chr) {
+                // check for dots expansion only at path component boundaries
+                handle_dots_push(&mut expanded, dots_count);
+                dots_count = 0;
+            } else {
+                // got non-dot within path component => do not expand any dots
+                while dots_count > 0 {
+                    expanded.push('.');
+                    dots_count -= 1;
+                }
+            }
+            expanded.push(chr);
         }
     }
 
@@ -70,21 +95,81 @@ pub fn expand_path<'a>(path: &'a str) -> Cow<'a, str> {
 mod tests {
     use super::*;
 
+    // common tests
     #[test]
     fn string_without_ndots() {
         assert_eq!("../hola", &expand_ndots("../hola").to_string());
     }
 
     #[test]
-    fn string_with_three_ndots() {
-        assert_eq!("../..", &expand_ndots("...").to_string());
+    fn string_with_three_ndots_and_chars() {
+        assert_eq!("a...b", &expand_ndots("a...b").to_string());
     }
 
+    #[test]
+    fn string_with_two_ndots_and_chars() {
+        assert_eq!("a..b", &expand_ndots("a..b").to_string());
+    }
+
+    #[test]
+    fn string_with_one_dot_and_chars() {
+        assert_eq!("a.b", &expand_ndots("a.b").to_string());
+    }
+
+    // Windows tests
+    #[cfg(windows)]
+    #[test]
+    fn string_with_three_ndots() {
+        assert_eq!(r"..\..", &expand_ndots("...").to_string());
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn string_with_mixed_ndots_and_chars() {
+        assert_eq!(
+            r"a...b/./c..d/../e.f/..\..\..//.",
+            &expand_ndots("a...b/./c..d/../e.f/....//.").to_string()
+        );
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn string_with_three_ndots_and_final_slash() {
+        assert_eq!(r"..\../", &expand_ndots(".../").to_string());
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn string_with_three_ndots_and_garbage() {
+        assert_eq!(
+            r"ls ..\../ garbage.*[",
+            &expand_ndots("ls .../ garbage.*[").to_string(),
+        );
+    }
+
+    // non-Windows tests
+    #[cfg(not(windows))]
+    #[test]
+    fn string_with_three_ndots() {
+        assert_eq!(r"../..", &expand_ndots("...").to_string());
+    }
+
+    #[cfg(not(windows))]
+    #[test]
+    fn string_with_mixed_ndots_and_chars() {
+        assert_eq!(
+            "a...b/./c..d/../e.f/../../..//.",
+            &expand_ndots("a...b/./c..d/../e.f/....//.").to_string()
+        );
+    }
+
+    #[cfg(not(windows))]
     #[test]
     fn string_with_three_ndots_and_final_slash() {
         assert_eq!("../../", &expand_ndots(".../").to_string());
     }
 
+    #[cfg(not(windows))]
     #[test]
     fn string_with_three_ndots_and_garbage() {
         assert_eq!(