Merge pull request #1917 from Xavrir/issue-1900

fix: handle invalid working directories gracefully with --full-path

Author: tmccombs

CHANGELOG.md

@@ -1,3 +1,8 @@
+# Unreleased
+
+## Bugfixes
+- Handle invalid working directories gracefully when using `--full-path`, see #1900 (@Xavrir).
+
 # 10.4.2
 
 ## Bugfixes

src/config.rs

@@ -15,9 +15,9 @@ pub struct Config {
     /// Whether the search is case-sensitive or case-insensitive.
     pub case_sensitive: bool,
 
-    /// Whether to search within the full file path or just the base name (filename or directory
-    /// name).
-    pub search_full_path: bool,
+    /// Cached current working directory for absolute path construction.
+    /// Populated when `--full-path` is set; `None` means search by filename only.
+    pub cwd: Option<PathBuf>,
 
     /// Whether to ignore hidden files and directories (or not).
     pub ignore_hidden: bool,

src/filesystem.rs

@@ -20,6 +20,17 @@ pub fn path_absolute_form(path: &Path) -> io::Result<PathBuf> {
     env::current_dir().map(|path_buf| path_buf.join(path))
 }
 
+/// Construct an absolute path from a potentially relative path and a
+/// pre-resolved working directory. Unlike `path_absolute_form`, this
+/// does not call `env::current_dir()` and cannot fail.
+pub fn make_absolute(path: &Path, cwd: &Path) -> PathBuf {
+    if path.is_absolute() {
+        return path.to_path_buf();
+    }
+    let path = path.strip_prefix(".").unwrap_or(path);
+    cwd.join(path)
+}
+
 pub fn absolute_path(path: &Path) -> io::Result<PathBuf> {
     let path_buf = path_absolute_form(path)?;
 
@@ -153,4 +164,40 @@ mod tests {
             Path::new("foo/bar/baz")
         );
     }
+
+    #[test]
+    fn make_absolute_with_relative_path() {
+        use super::make_absolute;
+        use std::path::PathBuf;
+
+        let cwd = Path::new("/home/user");
+        assert_eq!(
+            make_absolute(Path::new("foo/bar"), cwd),
+            PathBuf::from("/home/user/foo/bar")
+        );
+    }
+
+    #[test]
+    fn make_absolute_strips_dot_prefix() {
+        use super::make_absolute;
+        use std::path::PathBuf;
+
+        let cwd = Path::new("/home/user");
+        assert_eq!(
+            make_absolute(Path::new("./foo/bar"), cwd),
+            PathBuf::from("/home/user/foo/bar")
+        );
+    }
+
+    #[test]
+    fn make_absolute_with_absolute_path() {
+        use super::make_absolute;
+        use std::path::PathBuf;
+
+        let cwd = Path::new("/home/user");
+        assert_eq!(
+            make_absolute(Path::new("/absolute/path"), cwd),
+            PathBuf::from("/absolute/path")
+        );
+    }
 }

src/main.rs

@@ -245,9 +245,18 @@ fn construct_config(mut opts: Opts, pattern_regexps: &[String]) -> Result<Config
     let command = extract_command(&mut opts, colored_output)?;
     let has_command = command.is_some();
 
+    let cwd = if opts.full_path {
+        Some(env::current_dir().context(
+            "Could not determine current directory. \
+             This is required for --full-path.",
+        )?)
+    } else {
+        None
+    };
+
     Ok(Config {
         case_sensitive,
-        search_full_path: opts.full_path,
+        cwd,
         ignore_hidden: !(opts.hidden || opts.rg_alias_ignore()),
         read_fdignore: !(opts.no_ignore || opts.rg_alias_ignore()),
         read_vcsignore: !(opts.no_ignore || opts.rg_alias_ignore() || opts.no_ignore_vcs),

src/walk.rs

@@ -524,20 +524,7 @@ impl WorkerState {
                 // Check the name first, since it doesn't require metadata
                 let entry_path = entry.path();
 
-                let search_str: Cow<OsStr> = if config.search_full_path {
-                    let path_abs_buf = filesystem::path_absolute_form(entry_path)
-                        .expect("Retrieving absolute path succeeds");
-                    Cow::Owned(path_abs_buf.as_os_str().to_os_string())
-                } else {
-                    match entry_path.file_name() {
-                        Some(filename) => Cow::Borrowed(filename),
-                        None => unreachable!(
-                            "Encountered file system entry without a file name. This should only \
-                             happen for paths like 'foo/bar/..' or '/' which are not supposed to \
-                             appear in a file system traversal."
-                        ),
-                    }
-                };
+                let search_str = search_str_for_entry(entry_path, config.cwd.as_deref());
 
                 if !patterns
                     .iter()
@@ -676,6 +663,25 @@ impl WorkerState {
     }
 }
 
+fn search_str_for_entry<'a>(
+    entry_path: &'a std::path::Path,
+    cwd: Option<&std::path::Path>,
+) -> Cow<'a, OsStr> {
+    if let Some(cwd) = cwd {
+        let abs_path = filesystem::make_absolute(entry_path, cwd);
+        Cow::Owned(abs_path.into_os_string())
+    } else {
+        match entry_path.file_name() {
+            Some(filename) => Cow::Borrowed(filename),
+            None => unreachable!(
+                "Encountered file system entry without a file name. This should only \
+                 happen for paths like 'foo/bar/..' or '/' which are not supposed to \
+                 appear in a file system traversal."
+            ),
+        }
+    }
+}
+
 /// Recursively scan the given search path for files / pathnames matching the patterns.
 ///
 /// If the `--exec` argument was supplied, this will create a thread pool for executing