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

Combination of --no-ignore, --sort, and search path of a symbolic link to a directory causes panic #1389

Closed
HarrisonMc555 opened this issue Sep 25, 2019 · 4 comments

Comments

@HarrisonMc555
Copy link

HarrisonMc555 commented Sep 25, 2019

What version of ripgrep are you using?

ripgrep 11.0.2
-SIMD -AVX (compiled)
+SIMD +AVX (runtime)

How did you install ripgrep?

brew install ripgrep

What operating system are you using ripgrep on?

macOS Mojave 10.14.6 (18G95)

Describe your question, feature request, or bug.

When I run the following command that searches a symbolic link, ripgrep panics (after displaying output):

rg -n --hidden --no-ignore --color=always --sort path pattern ~/symbolic-link-to-directory

If this is a bug, what are the steps to reproduce the behavior?

Use the previous flags on a symbolic link to a directory.

If this is a bug, what is the actual behavior?

$ rg --debug -n --hidden --no-ignore --color=always --sort path every baz
DEBUG|rg::config|src/config.rs:40: /Users/harrisonmccullough/.ripgreprc: arguments loaded from config file: ["--smart-case"]
DEBUG|rg::args|src/args.rs:560: final argv: ["rg", "--smart-case", "--debug", "-n", "--hidden", "--no-ignore", "--color=always", "--sort", "path", "every", "baz"]
DEBUG|grep_regex::literal|grep-regex/src/literal.rs:59: literal prefixes detected: Literals { lits: [Complete(EVERY), Complete(eVERY), Complete(EvERY), Complete(evERY), Complete(EVeRY), Complete(eVeRY), Complete(EveRY), Complete(eveRY), Complete(EVErY), Complete(eVErY), Complete(EvErY), Complete(evErY), Complete(EVerY), Complete(eVerY), Complete(EverY), Complete(everY), Complete(EVERy), Complete(eVERy), Complete(EvERy), Complete(evERy), Complete(EVeRy), Complete(eVeRy), Complete(EveRy), Complete(eveRy), Complete(EVEry), Complete(eVEry), Complete(EvEry), Complete(evEry), Complete(EVery), Complete(eVery), Complete(Every), Complete(every)], limit_size: 250, limit_class: 10 }
DEBUG|globset|globset/src/lib.rs:435: built glob set; 0 literals, 0 basenames, 11 extensions, 0 prefixes, 0 suffixes, 0 required extensions, 0 regexes
baz/file2.txt
1:Hello everyone
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/libcore/option.rs:347:21
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: std::panicking::rust_panic_with_hook
   5: std::panicking::continue_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::panicking::panic
   9: rg::try_main
  10: rg::main
  11: std::rt::lang_start::{{closure}}
  12: std::panicking::try::do_call
  13: __rust_maybe_catch_panic
  14: std::rt::lang_start_internal
  15: main

If this is a bug, what is the expected behavior?

Not panic.

@HarrisonMc555
Copy link
Author

After further experimentation, it looks like it is the combination of the following:

  • The search path is a symbolic link to a directory
  • The --no-ignore flag is set
  • The --sort flag is set (with something other than none)

I used the --no-config flag and got the same results.

@BurntSushi
Copy link
Owner

Please provide a complete reproduction, as the issue template requests. Please provide commands to setup an identical corpus, for example.

@HarrisonMc555
Copy link
Author

mkdir mydir
ln -s mydir mylink
echo "text in file" >> mydir/file.txt
rg text --no-ignore --sort path # no search path is OK
rg text --no-ignore --sort path mydir # regulary directory is also OK
rg text --no-ignore --sort path mylink # symbolic link to directory is NOT OK

If you either remove all ripgrep configuration files or include the --no-config flag you get the same results.

@HarrisonMc555 HarrisonMc555 changed the title Panics for unknown reason Combination of --no-ignore, --sort, and search path of a symbolic link to a directory causes panic Sep 25, 2019
nikofil added a commit to nikofil/ripgrep that referenced this issue Oct 2, 2019
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.

Fixes BurntSushi#1389

Signed-off-by: Nikos Filippakis <nikolaos.filippakis@cern.ch>
@nikofil
Copy link

nikofil commented Oct 2, 2019

Hey! I wanted to contribute to ripgrep as I use it a lot, so I made a PR to fix this issue. Hope that's ok!

This was caused by

ripgrep/ignore/src/walk.rs

Lines 1036 to 1040 in 8892bf6

if dent.file_type().is_dir() {
self.depth += 1;
Some(Ok(WalkEvent::Dir(dent)))
} else {
Some(Ok(WalkEvent::File(dent)))
where you get whether a file is a directory or not using is_dir(). When symlinks are not followed, however, this will return false even in the case of a symlink that points to a directory. Then, when ripgrep gets an event signaling that the directory listing is over

ripgrep/ignore/src/walk.rs

Lines 949 to 951 in 8892bf6

Ok(WalkEvent::Exit) => {
self.ig = self.ig.parent().unwrap();
}
it tries to get the parent Ignore object which doesn't exist in this case, causing a panic.

BurntSushi added a commit that referenced this issue Feb 17, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants