RT #36539: Edge cases in "find_perl" algorithms

[James E Keenan]

I am writing to this list concerning a ticket in the Perl 5 bug queue, 
https://rt.perl.org/rt3//Ticket/Display.html?id=36539: Edge cases in 
"find_perl" algorithms.  This ticket was filed in 2005 by Michael 
Schwern and I have pasted its original post below.

There were several rounds of back-and-forth between Schwern and Ken 
Williams, but the conversation petered out years ago.  We would like to 
close this ticket.  In that RT ticket, I asked whether this issue would 
be better handled by the so-called Perl Toolchain Gang; Leon Timmermans 
agreed that it would.  So I'm now writing to see where this should be 
reported properly.

Is there a particular bug tracker to which I could transfer this 
discussion?  (Note: I have no opinion myself on these issues, so I'm 
only looking for a procedural answer, not a substantive discussion.)

Thank you very much.
Jim Keenan


###########
Several places try to convert $^X to an absolute path. CPAN->perl,
ExtUtils::MM_Unix->find_perl and Module::Build all do this. CPANPLUS is
generally not affected as it lets either Module::Build or the shell figure
it out.

The search order is typically some variant on:

my @perls = ($^X, 'perl', 'perl5', "perl$]");
my @paths = (File::Spec->path, $Config{binexp});

foreach my $perl (@perls) {
foreach my $path (@paths) {
...
}
}

In the case when $^X cannot be found this algorithm is likely to find not
just the wrong perl but the wrong version of Perl. This is because it
will look for "perl" before "perl5" and "perl5" before "perl5.00504".
I've had it accidentally pick up perl1 before! The order should be
reversed, looking for the more version specific names before the generic
ones.

Additionally, since 5.6 perl binaries have been named "perl5.6.1" and not
"perl5.00601". This algorithm does not search for that.

So the @perls list should be:

@perls = ($^X);
push @perls, sprintf "perl%vd", $^V if defined $^V;
push @perls, ("perl$]", "perl5", "perl");

Module::Build and CPANPLUS do not appear to be vulnerable to this as they
only check for $^X and do not try any fallback filenames. CPANPLUS doesn't
even try to make $^X absolute and leaves that up to the shell or
Module::Build. This is probably safer as if it cannot find your perl it
will yelp rather than silently risk running the wrong version. Changing
CPAN and MakeMaker's behaviors at this point isn't worth it, but a warning
wouldn't hurt.

Additionally, CPAN looks for $^X in the cwd but does it wrong. The
algorithm should be something like this:

# $^X is absolute
if( File::Spec->file_name_is_absolute($^X) ) {
push @perls, $^X;
}
else {
my $first_dir = File::Spec->splitdir($^X))[0];

# $^X is ./path/to/perl or ../path/to/perl. Make it
# absolute using the cwd.
if( $first_dir eq File::Spec->curdir or
$first_dir eq File::Spec->updir )
{
push @perls, File::Spec->rel2abs($^X);
}
}
# else leave $^X alone and do a PATH search

This most closely simulates how a shell finds perl. CPAN.pm makes the
mistake of always looking for $^X in the cwd. This means it can get the
following wrong:

$ ls ./perl
./perl
$ perl -MCPAN -e shell

$^X will be "perl". It will look for "$cwd/perl" and find the one in the
cwd rather than perform a PATH search.

MakeMaker, CPANPLUS and Module::Build do not handle this case at all, 
they do
not look in the cwd. In this case MakeMaker and CPANPLUS will use the
relative $^X, which is dangerous because it will go wrong as soon as
something chdirs.

Module::Build has a heuristic which checks to see if the perl it has found
is the same as the perl it was run with so though it will not find the
perl in the cwd it will not be fooled by ones later in the search. The
heuristic is pretty simple: it compares the output of Config::myconfig.

To summarize:

Change the filename search order to look for the most specific
versions first. (Module::Build and CPANPLUS not affected)

Add perlX.Y.Z to the filename search. (MB and CP not affected)

Add a warning when we cannot find $^X and must fall back to
another filename. (MB and CP not affected)

Look for "$cwd/$^X" only when $^X is ./perl or ../perl.
(all affected)

Compare myconfig of the found perl and the perl we were run with
to better ensure we found the right Perl. (MB already does this.
CP not affected as it does not search for Perl)

I'll patch up MakeMaker to do this and provide a patch for CPAN.pm. If I'm
feeling gung-ho I might do CPANPLUS and Module::Build, too.