Re: Add on_trusted_init and on_untrusted_init to plperl [PATCH] - Mailing list pgsql-hackers

From Tim Bunce
Subject Re: Add on_trusted_init and on_untrusted_init to plperl [PATCH]
Date
Msg-id 20100128200142.GJ38673@timac.local
Whole thread Raw
In response to Re: Add on_trusted_init and on_untrusted_init to plperl [PATCH]  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Add on_trusted_init and on_untrusted_init to plperl [PATCH]  ("David E. Wheeler" <david@kineticode.com>)
List pgsql-hackers
On Thu, Jan 28, 2010 at 11:54:08AM -0500, Tom Lane wrote:
> Tim Bunce <Tim.Bunce@pobox.com> writes:
> > I think the only controversial change is this one:
> 
> >> - Adds plperl.on_trusted_init and plperl.on_untrusted_init GUCs
> >> Both are PGC_USERSET.
> >> SPI functions are not available when the code is run.
> >> Errors are detected and reported as ereport(ERROR, ...)
> > +     plperl.on_trusted_init runs inside the Safe compartment.
> 
> Isn't it a security hole if on_trusted_init is USERSET?  That means
> an unprivileged user can determine what will happen in plperlu.
> SUSET would be saner.

Yes. Good point (though ITYM on_UNtrusted_init). I'll change that.

> > As I recall, Tom had concerns over the combination of PGC_USERSET and
> > before-first-use semantics.
> 
> IIRC, what I was unhappy about was exposing shared library load as a
> time when interesting-to-the-user things would happen.  It looks like
> you have got rid of that and instead made it happen at the first call
> of a plperl or plperlu function (respectively).  That seems like a
> fairly reasonable way to work, and if it's defined that way, there
> doesn't seem any reason not to allow them to be USERSET/SUSET.

Great.

> But it ought to be documented like that, not with vague phrases like
> "when the interpreter is initialized" --- that means nothing to users.

I'll change that.

> One issue that ought to be mentioned is what about transaction
> rollback.  One could argue that if the first plperl call is inside
> a transaction that later rolls back, then all the side effects of that
> should be undone.  But we have no way to undo whatever happened inside
> perl, and at least in most likely uses of on_init there wouldn't
> be any advantage in trying to force a redo anyway.  I think it's okay
> to just define it as happening once regardless of any transaction
> failure (but of course this had better be documented).

Will do.

Once the previous patch lands I'll post an update to this patch with
those changes applied.

Thanks.

Tim.


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Review: Typed Table
Next
From: Josh Berkus
Date:
Subject: Re: Streaming replication, and walsender during recovery