Front page | perl.perl5.porters |
Postings from July 2012
Re: the "require" branch, maintperl, and security
Thread Previous
|
Thread Next
From:
Nicholas Clark
Date:
July 26, 2012 02:50
Subject:
Re: the "require" branch, maintperl, and security
Message ID:
20120726095006.GC9583@plum.flirble.org
On Wed, Jul 25, 2012 at 09:24:13PM -0000, Father Chrysostomos wrote:
> rjbs wrote:
> > The smoke-me/require branch contains a fix for this bug, and will land on
> > blead shortly assuming further testing finds no problems.
>
> That fix does not look correct to me.
>
> Why should require ::foo be treated as require foo, but not ::::foo or
> main::foo?
As to ::foo vs ::::foo, because they are not the same:
$ perl -le 'sub foo::bar {warn "here"}; $a = bless [], "::foo"; $a->bar'
here at -e line 1.
$ perl -le 'sub foo::bar {warn "here"}; $a = bless [], ::foo; $a->bar'
here at -e line 1.
vs
$ perl -le 'sub foo::bar {warn "here"}; $a = bless [], "::::foo"; $a->bar'
Can't locate object method "bar" via package "::::foo" at -e line 1.
$ perl -le 'sub foo::bar {warn "here"}; $a = bless [], ::::foo; $a->bar'
Can't locate object method "bar" via package "::::foo" at -e line 1.
> Also, require ::::foo produces this error message:
>
> $ ./miniperl -Ilib -e 'require ::::foo'
> Bareword in require maps to disallowed filename "/foo.pm" at -e line 1.
>
> But perldiag says this:
>
> (F) The bareword form of require has been invoked with a filename which could
> not have been generated by a valid bareword permitted by the parser. You
> shouldn't be able to get this error from Perl code, but XS code may throw it
> if it passes an invalid module name to C<Perl_load_module>.
That's a good catch.
> I suggest not applying the s/::foo/foo/ patch and changing perldiag.
I'm not (yet) comfortable with that, because other parts of the language
treat ::foo and foo as equivalent to each other, but not equivalent to
::::foo
> Does the file name validation have to be a run-time check? Cannot it
> go in ck_require? Yes, I am suggesting making require ::foo a com-
> pile-time error, but that does not break any more code than making it
> an error at run time, since it was not an error before. Making it a
> compile-time error makes pp_require faster, and keeps the bareword-
> handling code in just two places, instead of three.
Speed wise it makes no difference, as it's already code called only once at
runtime, because it's after the lookup on %INC. For a successful require, the
second and subsequent calls at runtime will have already returned due to
this:
SV * const * const svp = hv_fetch(GvHVn(PL_incgv),
unixname, unixlen, 0);
if ( svp ) {
if (*svp != &PL_sv_undef)
RETPUSHYES;
else
DIE(aTHX_ "Attempt to reload %s aborted.\n"
"Compilation failed in require", unixname);
}
Complexity wise, it's harder to do it in ck_require (and be sure that it's
correct) because there's some platform specific special case code in
pp_require which can end up generating absolute paths, which would have to
be duplicated in ck_require. Even if the code is factored out, that's
actually doing more work than this way, because it ends up being called
twice - once at compile time, and once at run time.
The earlier iteration of the patch tried to do it that way, and it seemed
to be more complex, and have possible holes due to it not being obvious
that ck_require and (post manipulation) pp_require had identical ideas
about exactly which pathname a package name would map to (and hence whether
it was suspect).
Nicholas Clark
Thread Previous
|
Thread Next