beyondgrep / ack2

**ack 2 is no longer being maintained. ack 3 is the latest version.**
https://github.com/beyondgrep/ack3/
Other
1.48k stars 140 forks source link

check for unreadable filenames may fail with -r _ / standard filetest #313

Closed markuschwa closed 10 years ago

markuschwa commented 11 years ago

We have to use a ack on a network filesystem where the file permissions are reported wrongly when using stat(2) or ls -l.

For this reason ack (ack-2.04-single-file) sometimes skips a lot of files as unreadble which are actually readable in line 192.

The following patch fixes this:

--- ack-2.04-single-file    2013-07-26 15:18:13.000000000 +0200
+++ /usr/bin/ack    2013-08-12 17:22:36.229526915 +0200
@@ -190,7 +195,8 @@
         return 0 if -p $File::Next::name;

         # we can't handle unreadable filenames; report them
-        unless ( -r _ ) {
+        use filetest 'access'; 
+        unless ( -r $File::Next::name ) {
             if ( $App::Ack::report_bad_filenames ) {
                 App::Ack::warn( "${File::Next::name}: cannot open file for reading" );
             }

See also: perldoc.perl.org/filetest.html

M.S.

petdance commented 11 years ago

To consider:

markuschwa commented 11 years ago

What about re-checking only when -r _ fails?

    unless ( -r _ ) {
        use filetest 'access';  
        unless ( -r $File::Next::name ) {
            if ( $App::Ack::report_bad_filenames ) {
                App::Ack::warn( "${File::Next::name}: cannot open file for reading" );
            }
            return 0;
        }
    }

Additionally, this behavior could be made an option.

jonathanperret commented 11 years ago

I just hit this problem, working with ACLs on OS X:

$ echo astring > afile
$ chmod a-r afile
$ chmod +a "$USER allow read" afile
$ ack astring
ack: afile: cannot open file for reading
$ cat afile
astring

The patch above almost fixes the problem ; I have to use -R instead of -r:

unless ( -r _ ) {
    use filetest 'access';  
    unless ( -R $File::Next::name ) {
        if ( $App::Ack::report_bad_filenames ) {
            App::Ack::warn( "${File::Next::name}: cannot open file for reading" );
        }
        return 0;
    }
}
petdance commented 10 years ago

Why does the -R work but not -r?

petdance commented 10 years ago

This is applied in 5db825e60641269518783656418357186bcc13d6. @markuschwa, if you give me your name I'll add you to the Changes file and acknowledgements.

jonathanperret commented 10 years ago

Hi, thank you for including @markuschwa's patch.

Honestly, it is not entirely clear to me why I had to use -R instead of -r.

perldoc perlfunc says -R uses the real uid instead of the effective uid.

man access (http://linux.die.net/man/2/access) says :

The check is done using the calling process's real UID and GID, rather than the effective IDs as is done when actually attempting an operation (e.g., open(2)) on the file. This allows set-user-ID programs to easily determine the invoking user's authority.

So at least using -R with filetest 'access' sounds consistent, since access doesn't seem to promise anything else but checking with the real uid (EDIT: having looked at the code, Perl does seem to try a few tricks to emulate an "effective uid access" even when the OS does not provide eaccess. Apparently it did not work in my case.).

But really, it just worked in my ACL use case with -R and not with -r, so that's what I reported. Someone using sudo ack might get surprises, I guess.