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

pod2usage can invoke system($Config{pager},...) which is "less -R" => error #16246

Closed
p5pRT opened this issue Nov 14, 2017 · 10 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented Nov 14, 2017

Migrated from rt.perl.org#132446 (status was 'open')

Searchable as RT132446$

@p5pRT
Copy link
Author

p5pRT commented Nov 14, 2017

From karl@freefriends.org

This is a bug report for perl from karl@​freefriends.org,
generated with the help of perlbug 1.40 running under perl 5.26.1.

With stock perl 5.26.1, installed from source, I got this error message
from the pod2usage function​:

  Can't exec "/usr/bin/less -R"​: No such file or directory at
  /usr/local/lib/perl5/5.26.1/Pod/Usage.pm line 164.

I was running "tlmgr help | head", where tlmgr is the "TeX Live manager"
program (tug.org/texlive), but I think any invocation with -noperldoc
will suffice. Sorry, I don't have a minimal example at hand.

That line 164 is​:
  system(($Config{pager} || $ENV{PAGER} || '/bin/more'), $1);

With 5.26.1, my $Config{pager} is "less -R". system takes that string
literally as the command name, not breaking it at spaces, because it's
being called with the list form. Hence the failure.

In an earlier perl (5.16.3, distributed as the system version with
current CentOS7), $Config{pager} is "less -isr". Same problem.

I compiled 5.26.1 myself and did nothing special about the pager value;
perl somehow determined the "less -R" itself.

I don't know if it is the $Config{pager} value that should be a simple
command name, or Pod​::Usage at fault for assuming it is such.

Also on that line​: it seems like the user's value $ENV{PAGER} should be
used in preference to Perl's value?

Thanks for Perl. --karl

P.S. The line before that says​:
  # RT16091​: fall back to more if perldoc failed
but enter 16091 at rt.perl.org says "could not load bug",
and at rt.cpan.org goes to something unrelated.


Flags​:
  category=library
  severity=medium
  module=Pod​::Usage


Site configuration information for perl 5.26.1​:

Configured by karl at Tue Oct 31 22​:13​:49 CET 2017.

Summary of my perl5 (revision 5 version 26 subversion 1) configuration​:
 
  Platform​:
  osname=linux
  osvers=3.10.0-693.5.2.el7.x86_64
  archname=x86_64-linux
  uname='linux tug.org 3.10.0-693.5.2.el7.x86_64 #1 smp fri oct 20 20​:32​:50 utc 2017 x86_64 x86_64 x86_64 gnulinux '
  config_args='-d -e -Uinstallusrbinperl -Dprefix=/usr/local'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=undef
  usemultiplicity=undef
  use64bitint=define
  use64bitall=define
  uselongdouble=undef
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
  optimize='-O2'
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='4.8.5 20150623 (Red Hat 4.8.5-16)'
  gccosandvers=''
  intsize=4
  longsize=8
  ptrsize=8
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=16
  longdblkind=3
  ivtype='long'
  ivsize=8
  nvtype='double'
  nvsize=8
  Off_t='off_t'
  lseeksize=8
  alignbytes=8
  prototype=define
  Linker and Libraries​:
  ld='cc'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib /lib/../lib64 /usr/lib/../lib64 /lib /lib64 /usr/lib64 /usr/local/lib64
  libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.17.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.17'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E'
  cccdlflags='-fPIC'
  lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'


@​INC for perl 5.26.1​:
  /usr/local/lib/perl5/site_perl/5.26.1/x86_64-linux
  /usr/local/lib/perl5/site_perl/5.26.1
  /usr/local/lib/perl5/5.26.1/x86_64-linux
  /usr/local/lib/perl5/5.26.1
  /usr/local/lib/perl5/site_perl/5.24.1
  /usr/local/lib/perl5/site_perl/5.24.0
  /usr/local/lib/perl5/site_perl


Environment for perl 5.26.1​:
  HOME=/home/karl
  LANG (unset)
  LANGUAGE (unset)
  LC_ALL=C
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=.​:/home/karl/bin​:/l/bin​:/usr/local/gnu/emacs21/bin​:/usr/local/gnu/bin​:/home/texlive/karl/Master/bin/x86_64-linux​:/home/texlive/karl/Master/tlpkg/bin​:/usr/local/bin​:/usr/bin​:/usr/sbin​:/usr/lib/mailman/bin
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Nov 15, 2017

From @jkeenan

On Tue, 14 Nov 2017 18​:30​:30 GMT, karl@​freefriends.org wrote​:

This is a bug report for perl from karl@​freefriends.org,
generated with the help of perlbug 1.40 running under perl 5.26.1.

With stock perl 5.26.1, installed from source, I got this error
message
from the pod2usage function​:

Can't exec "/usr/bin/less -R"​: No such file or directory at
/usr/local/lib/perl5/5.26.1/Pod/Usage.pm line 164.

Do you in fact not have a program '/usr/bin/less'?

I was running "tlmgr help | head", where tlmgr is the "TeX Live
manager"
program (tug.org/texlive), but I think any invocation with -noperldoc
will suffice. Sorry, I don't have a minimal example at hand.

In the absence of an example we'll have trouble responding to this. pod2usage works well for (no warnings or exceptions).

Also, AFAICT, '-noperldoc' is a possible switch for the function Pod​::Usage​::pod2usage() but is *not* a possible switch for the executable program 'pod2usage'.

That line 164 is​:
system(($Config{pager} || $ENV{PAGER} || '/bin/more'), $1);

With 5.26.1, my $Config{pager} is "less -R". system takes that string
literally as the command name, not breaking it at spaces, because it's
being called with the list form. Hence the failure.

In an earlier perl (5.16.3, distributed as the system version with
current CentOS7), $Config{pager} is "less -isr". Same problem.

I compiled 5.26.1 myself and did nothing special about the pager
value;
perl somehow determined the "less -R" itself.

I don't know if it is the $Config{pager} value that should be a simple
command name, or Pod​::Usage at fault for assuming it is such.

Also on that line​: it seems like the user's value $ENV{PAGER} should
be
used in preference to Perl's value?

Thanks for Perl. --karl

P.S. The line before that says​:
# RT16091​: fall back to more if perldoc failed
but enter 16091 at rt.perl.org says "could not load bug",
and at rt.cpan.org goes to something unrelated.

In addition, Pod​::Usage is maintained upstream on CPAN. So if there really is a bug here (I'm not convinced yet), it will have to be reported here​: https://rt.cpan.org/Dist/Display.html?Name=Pod-Usage

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Nov 15, 2017

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Nov 15, 2017

From zefram@fysh.org

James E Keenan via RT wrote​:

Do you in fact not have a program '/usr/bin/less'?

I think he does have the file '/usr/bin/less', but not the file
'/usr/bin/less -R'. The question is whether $Config{pager} is a filename
or a fragment of shell code.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Nov 16, 2017

From @xsawyerx

On 11/15/2017 05​:41 PM, Zefram wrote​:

James E Keenan via RT wrote​:

Do you in fact not have a program '/usr/bin/less'?
I think he does have the file '/usr/bin/less', but not the file
'/usr/bin/less -R'. The question is whether $Config{pager} is a filename
or a fragment of shell code.

What would be setting $Config{pager} to shell code?

My inclination is to view these as paths and not shell code, but I'm not
sure what it is setting this and whether this is the way it has been
done thusfar.

@p5pRT
Copy link
Author

p5pRT commented Nov 16, 2017

From @Leont

On Thu, Nov 16, 2017 at 12​:22 PM, Sawyer X <xsawyerx@​gmail.com> wrote​:

On 11/15/2017 05​:41 PM, Zefram wrote​:

James E Keenan via RT wrote​:

Do you in fact not have a program '/usr/bin/less'?
I think he does have the file '/usr/bin/less', but not the file
'/usr/bin/less -R'. The question is whether $Config{pager} is a filename
or a fragment of shell code.

What would be setting $Config{pager} to shell code?

My inclination is to view these as paths and not shell code, but I'm not
sure what it is setting this and whether this is the way it has been
done thusfar.

Setting -R is necessary to support ANSI color codes, which is necessary to
support perldoc-without-man.

IMHO the preferred solution here is to pass $Config{pager} through
Text​::ParseWords's shellwords. Alternatively, we could add a pagerflags
entry to %Config, but I'm not convinced that that would be the path of
least resistance.

Leon

@p5pRT
Copy link
Author

p5pRT commented Nov 16, 2017

From @xsawyerx

On 11/16/2017 03​:14 PM, Leon Timmermans wrote​:

On Thu, Nov 16, 2017 at 12​:22 PM, Sawyer X <xsawyerx@​gmail.com
<mailto​:xsawyerx@​gmail.com>> wrote​:

On 11/15/2017 05&#8203;:41 PM\, Zefram wrote&#8203;:
> James E Keenan via RT wrote&#8203;:
>> Do you in fact not have a program '/usr/bin/less'?
> I think he does have the file '/usr/bin/less'\, but not the file
> '/usr/bin/less \-R'\.  The question is whether $Config\{pager\} is a
filename
> or a fragment of shell code\.

What would be setting $Config\{pager\} to shell code?

My inclination is to view these as paths and not shell code\, but
I'm not
sure what it is setting this and whether this is the way it has been
done thusfar\.

Setting -R is necessary to support ANSI color codes, which is
necessary to support perldoc-without-man.

If I understand you correctly, perldoc is actually using a different
mechanism by running it with "-R" manually.

@p5pRT
Copy link
Author

p5pRT commented Nov 17, 2017

From karl@freefriends.org

1) I'll be happy to send a report to bug-Pod-Usage if that is desired.
Or maybe you can just redirect this RT ticket there? Whatever works.

Meanwhile, to answer some of the questions that have been asked​:

2) It is perl's Configure which is setting $Config{pager} to
"/usr/bin/less -R", including the argument. (If you search for the
literal string "$less" in Configure, you will see.)

Configure has evidently been doing this since perl 5.12 (ca.2011).
Perl 5.10.1 sets it to just "/full/path/name/less", without options.
I am not passing any special arguments to Configure.
FWIW, example command that I used to inspect​:
  ./perl5.10.1 -e 'use Config; print $Config{pager}, "\n"'

2) The bug is evident. To repeat, line 164 of current Pod/Usage.pm is​:
  system(($Config{pager} || $ENV{PAGER} || '/bin/more'), $1);
which assumes $Config{pager} is merely the executable name, because
it's calling system() with a list. Thus whitespace is not parsed
into words. I can construct an example if I must.

3) This code in Pod/Usage.pm has not changed since at least 5.10.1. Thus
the assumption that $Config{pager} was a simple string was valid when it
was written, and the Configure script changed. FWIW, command I used to inspect​:
  grep 'Config{pager}' `locate Pod/Usage.pm`

4) I am nobody, but nevertheless, FWIW & IMHO, it seems a lot more
sensible to change Pod/Usage.pm, as Leon suggested, than to change
Configure back to making $Config{pager} being a bare pathname, merely
for this obscure case. That would surely break many more things which
rely on the current (and good) Configure behavior.

5) Independently, it still seems to me that, on that line, $ENV{PAGER}
should be preferred to the Perl-determined pager.

Thanks,
Karl

@kberry
Copy link

kberry commented Feb 16, 2020

I opened bugs with Pod::Usage at CPAN for these two issues. So I think this can be closed. (Apparently I don't have permission to do so.)
https://rt.cpan.org/Ticket/Display.html?id=131844
https://rt.cpan.org/Ticket/Display.html?id=131845

@jkeenan
Copy link
Contributor

jkeenan commented Feb 17, 2020

I opened bugs with Pod::Usage at CPAN for these two issues. So I think this can be closed. (Apparently I don't have permission to do so.)
https://rt.cpan.org/Ticket/Display.html?id=131844
https://rt.cpan.org/Ticket/Display.html?id=131845

Closing as Sent to CPAN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants