summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrew Gallant <jamslam@gmail.com>2020-02-17 15:59:03 -0500
committerAndrew Gallant <jamslam@gmail.com>2020-02-17 17:16:28 -0500
commit52d7f474206eb517c7284448777ed839c0cae8bb (patch)
tree27a3896de5ad5b648984be5e4e3beb57ccda44d4
parent75cbe88fa2993bcb12cbbddb32670cba30527716 (diff)
ignore: treat symbolic links to directories as directories
Due to how walkdir works if symlinks are not followed, symlinks to directories are seen as simple files by ripgrep. This caused a panic in some cases due to receiving a WalkEvent::Exit event without a corresponding WalkEvent::Dir event. This is fixed by looking at the metadata of the file in the case of a symlink to determine if it's a directory. We are careful to only do this stat check when the depth of the entry is 0, as this bug only impacts us when 1) we aren't following symlinks generally and 2) the user provides a symlinked directory that we do follow as a top-level path to search. Fixes #1389, Closes #1397
-rw-r--r--CHANGELOG.md2
-rw-r--r--ignore/src/walk.rs20
-rw-r--r--tests/regression.rs15
3 files changed, 36 insertions, 1 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 30736139..13aadf7a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -49,6 +49,8 @@ Bug fixes:
This was a serious performance regression in some cases.
* [BUG #1344](https://github.com/BurntSushi/ripgrep/issues/1344):
Document usage of `--type all`.
+* [BUG #1389](https://github.com/BurntSushi/ripgrep/issues/1389):
+ Fixes a bug where ripgrep would panic when searching a symlinked directory.
* [BUG #1445](https://github.com/BurntSushi/ripgrep/issues/1445):
ripgrep now respects ignore rules from .git/info/exclude in worktrees.
* [BUG #1485](https://github.com/BurntSushi/ripgrep/issues/1485):
diff --git a/ignore/src/walk.rs b/ignore/src/walk.rs
index 331268a3..b430981f 100644
--- a/ignore/src/walk.rs
+++ b/ignore/src/walk.rs
@@ -1043,7 +1043,7 @@ impl Iterator for WalkEventIter {
None => None,
Some(Err(err)) => Some(Err(err)),
Some(Ok(dent)) => {
- if dent.file_type().is_dir() {
+ if walkdir_is_dir(&dent) {
self.depth += 1;
Some(Ok(WalkEvent::Dir(dent)))
} else {
@@ -1791,6 +1791,24 @@ fn path_equals(dent: &DirEntry, handle: &Handle) -> Result<bool, Error> {
.map_err(|err| Error::Io(err).with_path(dent.path()))
}
+/// Returns true if the given walkdir entry corresponds to a directory.
+///
+/// This is normally just `dent.file_type().is_dir()`, but when we aren't
+/// following symlinks, the root directory entry may be a symlink to a
+/// directory that we *do* follow---by virtue of it being specified by the user
+/// explicitly. In that case, we need to follow the symlink and query whether
+/// it's a directory or not. But we only do this for root entries to avoid an
+/// additional stat check in most cases.
+fn walkdir_is_dir(dent: &walkdir::DirEntry) -> bool {
+ if dent.file_type().is_dir() {
+ return true;
+ }
+ if !dent.file_type().is_symlink() || dent.depth() > 0 {
+ return false;
+ }
+ dent.path().metadata().ok().map_or(false, |md| md.file_type().is_dir())
+}
+
/// Returns true if and only if the given path is on the same device as the
/// given root device.
fn is_same_file_system(root_device: u64, path: &Path) -> Result<bool, Error> {
diff --git a/tests/regression.rs b/tests/regression.rs
index 72f58590..29f15a27 100644
--- a/tests/regression.rs
+++ b/tests/regression.rs
@@ -747,6 +747,21 @@ rgtest!(r1334_crazy_literals, |dir: Dir, mut cmd: TestCommand| {
);
});
+// See: https://github.com/BurntSushi/ripgrep/issues/1389
+rgtest!(r1389_bad_symlinks_no_biscuit, |dir: Dir, mut cmd: TestCommand| {
+ dir.create_dir("mydir");
+ dir.create("mydir/file.txt", "test");
+ dir.link_dir("mydir", "mylink");
+
+ let stdout = cmd.args(&[
+ "test",
+ "--no-ignore",
+ "--sort", "path",
+ "mylink",
+ ]).stdout();
+ eqnice!("mylink/file.txt:test\n", stdout);
+});
+
// See: https://github.com/BurntSushi/ripgrep/pull/1446
rgtest!(r1446_respect_excludes_in_worktree, |dir: Dir, mut cmd: TestCommand| {
dir.create_dir("repo/.git/info");