Re: [HACKERS] Add on_plperl_init and on_plperlu_init to plperl UPDATE 3 [PATCH]

2010-02-08 Thread Tim Bunce
On Sun, Feb 07, 2010 at 08:25:33PM -0500, Andrew Dunstan wrote:
 Tim Bunce wrote:
 This is the third update to the fourth of the patches to be split out
 from the former 'plperl feature patch 1'.
 
 Changes in this patch:
 
 - Added plperl.on_plperl_init and plperl.on_plperlu_init GUCs
 Both are PGC_SUSET
 SPI functions are not available when the code is run.
 Errors are detected and reported as ereport(ERROR, ...)
 Corresponding documentation and tests for both.
 
 - Renamed plperl.on_perl_init to plperl.on_init
 
 - Improved state management of select_perl_context()
 An error during interpreter initialization will leave
 the state (interp_state etc) unchanged.
 
 - The utf8fix code has been greatly simplified.
 
 - More code comments re PGC_SUSET and no access to SPI functions.
 
 
 The docs on this patch need some cleaning up and expanding:
 
* The possible insecurity of %_SHARED needs expanding.

I tried. I couldn't decide how to expand what Tom Lane suggested
(http://archives.postgresql.org/message-id/1344.1265223...@sss.pgh.pa.us)
without it turning into a sprawling security tutorial.

So, since his suggestion seemed complete enough (albeit fairly vague),
I just used it almost verbatim.

Also, the PL/Tcl docs don't mention the issue at all and the PL/Python
docs say only The global dictionary GD is public data, available to all
Python functions within a session. Use with care.

The wording in the PL/Python docs seems better (available to all ...
use with care), so I've adopted the same kind of wording.

* The docs still refer to plperl.on_untrusted_init

Oops. Thanks. Fixed.

* plperl.on_plperl_init and plperl.on_plperlu_init can be documented
  together rather than repeating the same stuff almost word for word.

Ok. Done.

* extra examples for these two, and an explanation of why one might
  want to use each of the three on*init settings would be good.

plperl.on_init has an example that seems fairly self-explantory.
I've added one to the on_plperl_init/on_plperlu_init section that
also explains how a superuser can use ALTER USER ... SET  to set
a value for a non-superuser.

 I'll continue reviewing the patch, but these things at least need fixing.

I've an updated patch ready to go. I'll hold on to it for now.
Let me know if you have any more issues, or not.
Thanks.

Tim.

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


Re: [HACKERS] Add on_plperl_init and on_plperlu_init to plperl UPDATE 3 [PATCH]

2010-02-08 Thread Tim Bunce
On Mon, Feb 08, 2010 at 01:44:16PM +, Tim Bunce wrote:
 
  I'll continue reviewing the patch, but these things at least need fixing.

Here's an updated patch. The only changes relative to the previous
version are in the docs, as I outlined in the previous message.

I'll add it to the commitfest.

Tim.

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


Re: [HACKERS] Add on_plperl_init and on_plperlu_init to plperl UPDATE 3 [PATCH]

2010-02-08 Thread Tim Bunce
[Sigh. This email, unlike the previous, actually includes the patch.]

On Mon, Feb 08, 2010 at 01:44:16PM +, Tim Bunce wrote:
 
  I'll continue reviewing the patch, but these things at least need fixing.

Here's an updated patch. The only changes relative to the previous
version are in the docs, as I outlined in the previous message.

I'll add it to the commitfest.

Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 7018624..f742f0b 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*** $$ LANGUAGE plperl;
*** 748,753 
--- 748,759 
 literalreturn $_SHARED{myquote}-gt;($_[0]);/literal
 at the expense of readability.)
/para
+ 
+   para
+ The varname%_SHARED/varname variable and other global state within
+ the language is public data, available to all PL/Perl functions within a
+ session. Use with care.
+   /para
   /sect1
  
   sect1 id=plperl-trusted
*** CREATE TRIGGER test_valid_id_trig
*** 1044,1069 
  
variablelist
  
!  varlistentry id=guc-plperl-on-perl-init xreflabel=plperl.on_perl_init
!   termvarnameplperl.on_perl_init/varname (typestring/type)/term
indexterm
!primaryvarnameplperl.on_perl_init/ configuration parameter/primary
/indexterm
listitem
 para
!Specifies perl code to be executed when a perl interpreter is first initialized.
 The SPI functions are not available when this code is executed.
 If the code fails with an error it will abort the initialization of the interpreter
 and propagate out to the calling query, causing the current transaction
 or subtransaction to be aborted.
 /para
 para
!The perl code is limited to a single string. Longer code can be placed
!into a module and loaded by the literalon_perl_init/ string.
 Examples:
  programlisting
! plplerl.on_perl_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
! plplerl.on_perl_init = 'use lib /my/app; use MyApp::PgInit;'
  /programlisting
 /para
 para
--- 1050,1076 
  
variablelist
  
!  varlistentry id=guc-plperl-on-init xreflabel=plperl.on_init
!   termvarnameplperl.on_init/varname (typestring/type)/term
indexterm
!primaryvarnameplperl.on_init/ configuration parameter/primary
/indexterm
listitem
 para
!Specifies Perl code to be executed when a Perl interpreter is first initialized
!and before it is specialized for use by literalplperl/ or literalplperlu/.
 The SPI functions are not available when this code is executed.
 If the code fails with an error it will abort the initialization of the interpreter
 and propagate out to the calling query, causing the current transaction
 or subtransaction to be aborted.
 /para
 para
!The Perl code is limited to a single string. Longer code can be placed
!into a module and loaded by the literalon_init/ string.
 Examples:
  programlisting
! plplerl.on_init = '$ENV{NYTPROF}=start=no; require Devel::NYTProf::PgPLPerl'
! plplerl.on_init = 'use lib /my/app; use MyApp::PgInit;'
  /programlisting
 /para
 para
*** plplerl.on_perl_init = 'use lib /my/app
*** 1077,1082 
--- 1084,1129 
/listitem
   /varlistentry
  
+  varlistentry id=guc-plperl-on-plperl-init xreflabel=plperl.on_plperl_init
+   termvarnameplperl.on_plperl_init/varname (typestring/type)/term
+   termvarnameplperl.on_plperlu_init/varname (typestring/type)/term
+   indexterm
+primaryvarnameplperl.on_plperl_init/ configuration parameter/primary
+   /indexterm
+   indexterm
+primaryvarnameplperl.on_plperlu_init/ configuration parameter/primary
+   /indexterm
+   listitem
+para
+These parameters specify Perl code to be executed when the
+literalplperl/, or literalplperlu/ language is first used in a
+session.  Changes to these parameters after the corresponding language
+has been used will have no effect.
+The SPI functions are not available when this code is executed.
+Only superusers can change these settings.
+The Perl code in literalplperl.on_plperl_init/ can only perform trusted operations.
+/para
+para
+The effect of setting these parameters is very similar to executing a
+literalDO/ command with the Perl code before any other use of the
+language.  The parameters are useful when you want to execute the Perl
+code automatically on every connection, or when a connection is not
+interactive. The parameters can be used by non-superusers by having a
+superuser execute an literalALTER USER ... SET .../ command.
+For example:
+ programlisting
+ ALTER USER joe SET plplerl.on_plperl_init = '$_SHARED{debug} = 1';
+ 

Re: [HACKERS] Add on_plperl_init and on_plperlu_init to plperl UPDATE 3 [PATCH]

2010-02-07 Thread Andrew Dunstan


Tim Bunce wrote:

This is the third update to the fourth of the patches to be split out
from the former 'plperl feature patch 1'.

Changes in this patch:

- Added plperl.on_plperl_init and plperl.on_plperlu_init GUCs
Both are PGC_SUSET
SPI functions are not available when the code is run.
Errors are detected and reported as ereport(ERROR, ...)
Corresponding documentation and tests for both.

- Renamed plperl.on_perl_init to plperl.on_init

- Improved state management of select_perl_context()
An error during interpreter initialization will leave
the state (interp_state etc) unchanged.

- The utf8fix code has been greatly simplified.

- More code comments re PGC_SUSET and no access to SPI functions.

  



The docs on this patch need some cleaning up and expanding:

   * The possible insecurity of %_SHARED needs expanding.
   * The docs still refer to plperl.on_untrusted_init
   * plperl.on_plperl_init and plperl.on_plperlu_init can be documented
 together rather than repeating the same stuff almost word for word.
   * extra examples for these two, and an explanation of why one might
 want to use each of the three on*init settings would be good.

I'll continue reviewing the patch, but these things at least need fixing.

cheers

andrew



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


Re: [HACKERS] Add on_plperl_init and on_plperlu_init to plperl UPDATE 3 [PATCH]

2010-02-05 Thread Alex Hunsaker
On Fri, Feb 5, 2010 at 06:40, Tim Bunce tim.bu...@pobox.com wrote:
 This is the third update to the fourth of the patches to be split out
 from the former 'plperl feature patch 1'.

 Changes in this patch:

 - Added plperl.on_plperl_init and plperl.on_plperlu_init GUCs
    Both are PGC_SUSET
    SPI functions are not available when the code is run.
    Errors are detected and reported as ereport(ERROR, ...)
    Corresponding documentation and tests for both.

*sniffle* OK I think I agree with everyone else on setting this as
PGC_SUSET until we can either prove it can be USERSET or we can fix it
so we can check permissions on SET properly.  It seems if you really
wanted a user to be able to set it you should be able to define a
SECURITY DEFINER function that sets it to a string you pass in or
something.  Obviously not part of core postgres...

 - Renamed plperl.on_perl_init to plperl.on_init

*shrug* OK (I think there was still some flack on this var, but I
think its ok-- we can discuss that in a separate thread if people
still disagree :) )

 - Improved state management of select_perl_context()
    An error during interpreter initialization will leave
    the state (interp_state etc) unchanged.

 - The utf8fix code has been greatly simplified.

 - More code comments re PGC_SUSET and no access to SPI functions.

I like the doc changes and think the new comment about %_SHARED being
unsafe is good.

All looks good to me.  Ill mark it as Ready for Committer

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