Re: RT #36539: Edge cases in "find_perl" algorithms

I have long recommended Probe::Perl for this sort of thing (which was
extracted from M::B for people who needed it standalone).

I don't think the ticket is actionable.  It's a lot of smoke, but no fire.

David



On Sat, Sep 7, 2013 at 8:40 AM, James E Keenan <[email protected]> wrote:
> 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.



-- 
David Golden <[email protected]>
Take back your inbox! → http://www.bunchmail.com/
Twitter/IRC: @xdg