Prev: PG 9.0 and standard_conforming_strings
Next: Add on_perl_init and proper destruction to plperl UPDATE v3[PATCH]
From: Alex Hunsaker on 29 Jan 2010 22:07
On Mon, Jan 25, 2010 at 12:53, Tim Bunce <Tim.Bunce(a)pobox.com> wrote:
> - Added the 'warnings' pragma to the list of modules to load into Safe.
>  So plperl functions can now "use warnings;" - added test for that.

*yay*

> - Added 'use 5.008001;' to plc_perlboot.pl as a run-time check to
>  complement the configure-time check added by Tom Lane recently.

For the curious the above works quite nicely.:
=> do $$ 1; $$ language plperl;
ERROR: Perl v5.20.1 required--this is only v5.10.1, stopped at line 1.
BEGIN failed--compilation aborted at line 1.

A couple of comments. *note* I have not tested this as a whole yet
(due to rejects).

in plc_perboot.pl
+$funcsrc .= qq[ package main; undef *{'$name'}; *{'$name'} = sub {
$BEGIN $prolog $src } ];

Any thoughts on using a package other than main? Maybe something like
package PlPerl or what not? Not that I think it really matters...
But it might be nicer to see in say NYTprof ?

in plc_safe_ok.pl
+use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe);

Is there some reason not to use my here? The only reason I can think
of is you want the *_init gucs to be able to stick things into
@ShareIntoSafe and friends? And if so should we document that
somewhere (or was that in an earlier patch that i missed?)? Or does
vars do some other magic that im not seeing that we need here?

Also whats the use case for $SafeClass? Multiple versions of Safe.pm?
Other modules that are like Safe.pm? Im just curious...

Hrm I also dont see the point of the new "use Safe;" as we still eval
it in in plperl_safe_init() care to enlighten a lost soul?

Other than those really quite minor questions that are arguably me
nitpicking... It looks great to me.

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Alex Hunsaker on 30 Jan 2010 13:08
On Sat, Jan 30, 2010 at 07:51, Tim Bunce <Tim.Bunce(a)pobox.com> wrote:
> On Fri, Jan 29, 2010 at 08:07:30PM -0700, Alex Hunsaker wrote:
>> A couple of comments. *note* I have not tested this as a whole yet
>> (due to rejects).
>>
>> in plc_perboot.pl
>> +$funcsrc .= qq[ package main; undef *{'$name'}; *{'$name'} = sub {
>> $BEGIN $prolog $src } ];
>>
>> Any thoughts on using a package other than main?  Maybe something like
>> package PlPerl or what not?  Not that I think it really matters...
>> But it might be nicer to see in say NYTprof ?
>
> The only interface Safe provides for sharing things with the compartment
> shares them with the root package of the compartment which, from within
> the compartment, is 'main::'.
>
> It's an unfortunate limitation of Safe. I could have added more code to
> wordaround that but opted to keep it simple for this commitfest.

I thought it might be something like that.

>> in plc_safe_ok.pl
>> +use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe);
>>
>> Is there some reason not to use my here?  The only reason I can think
>> of is you want the *_init gucs to be able to stick things into
>> @ShareIntoSafe and friends?  And if so should we document that
>> somewhere (or was that in an earlier patch that i missed?)?
>
> It's a step towards providing an interface to give the DBA control over
> what gets loaded into the Safe compartment and what gets shared with it.
>
> I opted to not try to define a formal documented interface because I
> felt it was likely to be a source of controversy and/or bikeshedding.
> This late in the game I didn't want to take that chance.
>
> I'd rather a few brave souls get some experience with it as-as, and collect
> some good use-cases, before proposing a formal documented API.

Fine with me.

>> Also whats the use case for $SafeClass?  Multiple versions of Safe.pm?
>>  Other modules that are like Safe.pm? Im just curious...
>
> It's possible someone might want to use an alternative module.
> Most likely their own local subclass of Safe that adds extra features.
> I'd explored that when trying to integrate NYTProf.  It seemed worth
> at least making possible.
>
>> Hrm I also dont see the point of the new "use Safe;"  as we still eval
>> it in in plperl_safe_init() care to enlighten a lost soul?
>
> It just makes the file more self-contained for syntax checking:
>    perl -cw plc_safeboot.pl
>
>> Other than those really quite minor questions that are arguably me
>> nitpicking...  It looks great to me.
>
> Great, thanks Alex!

I marked it as ready, though ill add a comment that it depends on the
other patch to apply cleanly (and if that one gets rebased...
obviously this one probably will need to as well).

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Alex Hunsaker on 12 Feb 2010 16:30
On Fri, Feb 12, 2010 at 13:42, Andrew Dunstan <andrew(a)dunslane.net> wrote:
>
>
> Alex Hunsaker wrote:
>>>>
>>>> in plc_safe_ok.pl
>>>> +use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe);
>>>> ...the *_init gucs to be able to stick things into
>>>> @ShareIntoSafe and friends?
>
> I'm not sure it's fine with me.
>
> I'm a bit inclined to commit the piece of this patch that concerns the
> warnings pragma,

I think a sizable portion of the patch can be dropped if you do the
above. Namely the whole double init protection that got added in the
last round.

> and leave the rest for the next release, unless you can
> convince me real fast that we're not opening a back door here. If we're
> going to allow DBAs to add things to the Safe container, it's going to be up
> front or not at all, as far as I'm concerned.

I think backdoor is a bit extreme. Yes it could allow people who
can set the plperl.*_init functions to muck with the safe. As an
admin I could also do that by setting plperl.on_init and overloading
subs in the Safe namespace or switching out Safe.pm.

Anyway reasons I can come up for it are:
-its general so we can add other modules/pragmas easily in the future
-helps with the plperl/plperlu all or nothing situation
-starts to flesh out how an actual exposed (read documented) interface
should look for 9.1

.... I know Tim mentioned David as having some use cases (cc'ed)

So my $0.2 is I don't have any strong feelings for/against it. I
think if we could expose *something* (even if you can only get to it
in a C contrib module) that would be great. But I realize it might be
impractical at this stage in the game.

*shrug*

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Alex Hunsaker on 12 Feb 2010 20:17
On Fri, Feb 12, 2010 at 17:57, Andrew Dunstan <andrew(a)dunslane.net> wrote:
> Another objection is that discussion on this facility has been pretty scant.
> I think that's putting it mildly.

Well I can certainly attest to that seeing as how I spotted it purely
by review...

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Robert Haas on 12 Feb 2010 21:14
On Fri, Feb 12, 2010 at 3:42 PM, Andrew Dunstan <andrew(a)dunslane.net> wrote:
> I'm a bit inclined to commit the piece of this patch that concerns the
> warnings pragma, and leave the rest for the next release,

Based on the subsequent discussion on this thread, +1 for this
approach. Allowing use warnings sounds good; opening a potential
security hole sounds bad. Sounds like it would considerably simplify
the patch, too.

....Robert

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

 |  Next  |  Last
Pages: 1 2 3
Prev: PG 9.0 and standard_conforming_strings
Next: Add on_perl_init and proper destruction to plperl UPDATE v3[PATCH]