No output when --pager is a missing environment variable

[Smylers]

Ack isn't giving me any output when invoked directly with sudo:

$ echo bang | ack ^
bang
$ echo bang | sudo ack ^
$ sudo -s
# echo bang | ack ^

As you can see, it works with a root shell (even when sudo was used to get that), just not directly with sudo.

I can make the output appear under sudo by hacking App::Ack to print to stdout rather than $fh, changing this:

sub print { print {$fh} @_; return; }

to:

sub print { print @_; return; }

In the Perl debugger I get the same value for $fh both with and without sudo, namely:

  DB<7> x $fh
0  GLOB(0x20a7fd8)
   -> *App::Ack::$pager
         FileHandle({*App::Ack::$pager}) => fileno(3)

I've found this with ack 2.04 and 2.12, on Ubuntu 10.04 and 13.10.

Sorry I don't have time to dig into this further right now. If you can't reproduce this, let me know and I'll try installing from Git.

[hoelzro]

@Smylers Using ack built from git HEAD (b3617f1), this works fine for me (Arch Linux). What version of sudo do you have (I have 1.8.10p3)? Do you have any ackrc files (try ack --dump to see if you're not sure)? Could you try reproducing this using ack built from git?

Thanks for the bug report!

[Smylers]

try ack --dump

Ah, thanks — that shows the problem. My .ackrc has:

--pager
$PAGER

And sudo is cleansing my environment, so by the time ack runs, $PAGER evaluates to the empty string. So I can make it work by configuring sudo to allow PAGER through. And to be robust, for systems where I can't configure sudo, having .ackrc include a fallback:

--pager
${PAGER:=more}

The actual problem is that if --pager is something which evaluates in the shell to the empty string, ack's output just goes missing — looking exactly the same as if ack hadn't found any matches. You can reproduce this most easily with something like:

$ echo crunch_eth | ack --pager '$DOESNT_EXIST' ^

An explicit empty string is detected as a problem, and an appropriate message displayed:

$ echo crunch_eth | ack --pager='' ^
Option pager requires an argument
ack: Invalid option on command line

Though weirdly only with the supposedly-optional = sign; without that, it evades detection, but at least it fails in a way the user will notice:

 $ echo crunch_eth | ack --pager '' ^
Missing command in piped open at /usr/share/perl5/App/Ack.pm line 472.
ack: Unable to pipe to pager "": Broken pipe

But an empty environment variable doesn't give either of those, which is unfortunate — but I'm guessing this may not be something that Ack can detect.

[hoelzro]

@Smylers This is an interesting issue...I actually had to look at the code to figure out why your ackrc works at all! We don't do any special processing of what the value --pager is set to, so I was wondering why ack was interpreting it as a shell variable. It turns out that open uses a shell for the subprocess when using pipes and 3-argument mode, so that's an unexpected benefit (I'm not surprised, though, considering my PAGER is less -RM). Anyway, enough babblering on my part; I just found this behavior interesting.

I'll look into detecting the case that you found; nice catch!

[hoelzro]

Unfortunately, it seems that this is an issue with sh; sh -c '' exits with status 0, and the worst part is printing to a pipe to such a process seems to succeed in Perl.

[Smylers]

I actually had to look at the code to figure out why your ackrc works at all!

I have a vague recollection that when I set that up, it was something I tried on the off-chance, and was pleased that it worked; I think I was otherwise about to wrap ack with a shell script which invoked it with --pager $PAGER. Having already set the well-known and long-standing environment variable $PAGER to my pager of choice, I don't want to have to go around explaining it separately to every individual command.

It turns out that open uses a shell for the subprocess when using pipes and 3-argument mode, so that's an unexpected benefit

Unless it actually turns out to be an unexpected security risk, of course.

Having --pager not being interpreted by the shell would be fine, if there were an option which means ‘just take notice of $PAGER’ (use it if set, don't have a pager otherwise).

[hoelzro]

We actually did have a security hole due to --pager before ack 2.12, but it has since been fixed. Unfortunately, I don't know if we can change how --pager behaves, seeing as users can have their PAGER set to things like less -RM and expect it to continue to work. Sticking with the shell is probably the best option.

[hoelzro]

Seeing as this is a problem we can't really solve, I would like to close this issue. Any objections, @Smylers?

[Smylers]

Seeing as this is a problem we can't really solve, I would like to
close this issue. Any objections, @Smylers?

Let's at least document the behaviour — both the useful feature that
--pager is subject to shell interpretation, and the gotcha that if it
evaluates to an empty string your output just disappears.

I'm happy to submit a patch for that.

[hoelzro]

@Smylers I would be happy to merge such a patch =) regarding the wording about interpolation, you might want to mention that --pager is evaluated by the shell, so that users know that both 'less -RM' and '$PAGER' work

[ghost]

Is this issue waiting on a doc patch alone to be closed?

[Smylers]

Oh, apparently so. Apologies for not having done this, and thank you for the reminder.