From 37f11c308136b64f521abfb87bdebd3f9df7b9e9 Mon Sep 17 00:00:00 2001 From: Bruce Momjian <bruce@momjian.us> Date: Sun, 10 Jul 2005 04:56:55 +0000 Subject: [PATCH] This patch addresses the problem mentioned in the "process crash when a plpython function returns unicode" thread: http://archives.postgresql.org/pgsql-bugs/2005-06/msg00105.php In several places PL/Python was calling PyObject_Str() and then PyString_AsString() without checking if the former had returned NULL to indicate an error. PyString_AsString() doesn't expect a NULL argument, so passing one causes a segmentation fault. This patch adds checks for NULL and raises errors via PLy_elog(), which prints details of the underlying Python exception. The patch also adds regression tests for these checks. All tests pass on my Solaris 9 box running HEAD and Python 2.4.1. In one place the patch doesn't call PLy_elog() because that could cause infinite recursion; see the comment I added. I'm not sure how to test that particular case or whether it's even possible to get an error there: the value that the code should check is the Python exception type, so I wonder if a NULL value "shouldn't happen." This patch converts NULL to "Unknown Exception" but I wonder if an Assert() would be appropriate. The patch is against HEAD but the same changes should be applied to earlier versions because they have the same problem. The patch might not apply cleanly against earlier versions -- will the committer take care of little differences or should I submit different versions of the patch? Michael Fuhr --- src/pl/plpython/expected/plpython_error.out | 17 +++++++++++ .../plpython/expected/plpython_function.out | 22 +++++++++++++++ src/pl/plpython/expected/plpython_schema.out | 3 ++ src/pl/plpython/plpython.c | 21 ++++++++++++-- src/pl/plpython/sql/plpython_error.sql | 9 ++++++ src/pl/plpython/sql/plpython_function.sql | 28 +++++++++++++++++++ src/pl/plpython/sql/plpython_schema.sql | 4 ++- 7 files changed, 100 insertions(+), 4 deletions(-) diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out index 06043289fed..8a0b08fe163 100644 --- a/src/pl/plpython/expected/plpython_error.out +++ b/src/pl/plpython/expected/plpython_error.out @@ -19,3 +19,20 @@ SELECT valid_type('rick'); (1 row) +-- +-- Test Unicode error handling. +-- +SELECT unicode_return_error(); +ERROR: plpython: function "unicode_return_error" could not create return value +DETAIL: exceptions.UnicodeEncodeError: 'ascii' codec can't encode character u'\x80' in position 0: ordinal not in range(128) +INSERT INTO unicode_test (testvalue) VALUES ('test'); +ERROR: plpython: function "unicode_trigger_error" could not modify tuple +DETAIL: exceptions.UnicodeEncodeError: 'ascii' codec can't encode character u'\x80' in position 0: ordinal not in range(128) +SELECT unicode_plan_error1(); +WARNING: plpython: in function unicode_plan_error1: +DETAIL: plpy.Error: Unknown error in PLy_spi_execute_plan +ERROR: plpython: function "unicode_plan_error1" could not execute plan +DETAIL: exceptions.UnicodeEncodeError: 'ascii' codec can't encode character u'\x80' in position 0: ordinal not in range(128) +SELECT unicode_plan_error2(); +ERROR: plpython: function "unicode_plan_error2" could not execute plan +DETAIL: exceptions.UnicodeEncodeError: 'ascii' codec can't encode character u'\x80' in position 0: ordinal not in range(128) diff --git a/src/pl/plpython/expected/plpython_function.out b/src/pl/plpython/expected/plpython_function.out index cc1415dca8d..516d0576899 100644 --- a/src/pl/plpython/expected/plpython_function.out +++ b/src/pl/plpython/expected/plpython_function.out @@ -267,3 +267,25 @@ LANGUAGE plpythonu; CREATE OR REPLACE FUNCTION newline_crlf() RETURNS integer AS 'x = 100\r\ny = 23\r\nreturn x + y\r\n' LANGUAGE plpythonu; +-- +-- Unicode error handling +-- +CREATE FUNCTION unicode_return_error() RETURNS text AS ' +return u"\\x80" +' LANGUAGE plpythonu; +CREATE FUNCTION unicode_trigger_error() RETURNS trigger AS ' +TD["new"]["testvalue"] = u"\\x80" +return "MODIFY" +' LANGUAGE plpythonu; +CREATE TRIGGER unicode_test_bi BEFORE INSERT ON unicode_test + FOR EACH ROW EXECUTE PROCEDURE unicode_trigger_error(); +CREATE FUNCTION unicode_plan_error1() RETURNS text AS ' +plan = plpy.prepare("SELECT $1 AS testvalue", ["text"]) +rv = plpy.execute(plan, [u"\\x80"], 1) +return rv[0]["testvalue"] +' LANGUAGE plpythonu; +CREATE FUNCTION unicode_plan_error2() RETURNS text AS ' +plan = plpy.prepare("SELECT $1 AS testvalue1, $2 AS testvalue2", ["text", "text"]) +rv = plpy.execute(plan, u"\\x80", 1) +return rv[0]["testvalue1"] +' LANGUAGE plpythonu; diff --git a/src/pl/plpython/expected/plpython_schema.out b/src/pl/plpython/expected/plpython_schema.out index e94e7bbcf88..727e4b83d92 100644 --- a/src/pl/plpython/expected/plpython_schema.out +++ b/src/pl/plpython/expected/plpython_schema.out @@ -41,3 +41,6 @@ CREATE TABLE xsequences ( sequence text not null ) ; CREATE INDEX xsequences_pid_idx ON xsequences(pid) ; +CREATE TABLE unicode_test ( + testvalue text NOT NULL +); diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c index 3d7230ffca9..b1d47c77820 100644 --- a/src/pl/plpython/plpython.c +++ b/src/pl/plpython/plpython.c @@ -29,7 +29,7 @@ * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. * * IDENTIFICATION - * $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.64 2005/07/05 18:15:51 momjian Exp $ + * $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.65 2005/07/10 04:56:55 momjian Exp $ * ********************************************************************* */ @@ -517,6 +517,8 @@ PLy_modify_tuple(PLyProcedure * proc, PyObject * pltd, TriggerData *tdata, if (plval != Py_None && !tupdesc->attrs[atti]->attisdropped) { plstr = PyObject_Str(plval); + if (!plstr) + PLy_elog(ERROR, "function \"%s\" could not modify tuple", proc->proname); src = PyString_AsString(plstr); modvalues[i] = FunctionCall3(&proc->result.out.r.atts[atti].typfunc, @@ -774,6 +776,8 @@ PLy_function_handler(FunctionCallInfo fcinfo, PLyProcedure * proc) { fcinfo->isnull = false; plrv_so = PyObject_Str(plrv); + if (!plrv_so) + PLy_elog(ERROR, "function \"%s\" could not create return value", proc->proname); plrv_sc = PyString_AsString(plrv_so); rv = FunctionCall3(&proc->result.out.d.typfunc, PointerGetDatum(plrv_sc), @@ -2019,7 +2023,9 @@ PLy_spi_execute_plan(PyObject * ob, PyObject * list, long limit) char *sv; PyObject *so = PyObject_Str(list); - + if (!so) + PLy_elog(ERROR, "function \"%s\" could not execute plan", + PLy_procedure_name(PLy_curr_procedure)); sv = PyString_AsString(so); PLy_exception_set(PLy_exc_spi_error, "Expected sequence of %d arguments, got %d. %s", @@ -2044,6 +2050,9 @@ PLy_spi_execute_plan(PyObject * ob, PyObject * list, long limit) if (elem != Py_None) { so = PyObject_Str(elem); + if (!so) + PLy_elog(ERROR, "function \"%s\" could not execute plan", + PLy_procedure_name(PLy_curr_procedure)); sv = PyString_AsString(so); /* @@ -2531,7 +2540,13 @@ PLy_traceback(int *xlevel) else vstr = "Unknown"; - estr = PyString_AsString(eob); + /* + * I'm not sure what to do if eob is NULL here -- we can't call + * PLy_elog because that function calls us, so we could end up + * with infinite recursion. I'm not even sure if eob could be + * NULL here -- would an Assert() be more appropriate? + */ + estr = eob ? PyString_AsString(eob) : "Unknown Exception"; xstr = PLy_printf("%s: %s", estr, vstr); Py_DECREF(eob); diff --git a/src/pl/plpython/sql/plpython_error.sql b/src/pl/plpython/sql/plpython_error.sql index 2f0486fed92..b04b23ef95e 100644 --- a/src/pl/plpython/sql/plpython_error.sql +++ b/src/pl/plpython/sql/plpython_error.sql @@ -7,3 +7,12 @@ SELECT invalid_type_uncaught('rick'); SELECT invalid_type_caught('rick'); SELECT invalid_type_reraised('rick'); SELECT valid_type('rick'); + +-- +-- Test Unicode error handling. +-- + +SELECT unicode_return_error(); +INSERT INTO unicode_test (testvalue) VALUES ('test'); +SELECT unicode_plan_error1(); +SELECT unicode_plan_error2(); diff --git a/src/pl/plpython/sql/plpython_function.sql b/src/pl/plpython/sql/plpython_function.sql index 25ac388495a..e3ec2afe014 100644 --- a/src/pl/plpython/sql/plpython_function.sql +++ b/src/pl/plpython/sql/plpython_function.sql @@ -313,3 +313,31 @@ LANGUAGE plpythonu; CREATE OR REPLACE FUNCTION newline_crlf() RETURNS integer AS 'x = 100\r\ny = 23\r\nreturn x + y\r\n' LANGUAGE plpythonu; + +-- +-- Unicode error handling +-- + +CREATE FUNCTION unicode_return_error() RETURNS text AS ' +return u"\\x80" +' LANGUAGE plpythonu; + +CREATE FUNCTION unicode_trigger_error() RETURNS trigger AS ' +TD["new"]["testvalue"] = u"\\x80" +return "MODIFY" +' LANGUAGE plpythonu; + +CREATE TRIGGER unicode_test_bi BEFORE INSERT ON unicode_test + FOR EACH ROW EXECUTE PROCEDURE unicode_trigger_error(); + +CREATE FUNCTION unicode_plan_error1() RETURNS text AS ' +plan = plpy.prepare("SELECT $1 AS testvalue", ["text"]) +rv = plpy.execute(plan, [u"\\x80"], 1) +return rv[0]["testvalue"] +' LANGUAGE plpythonu; + +CREATE FUNCTION unicode_plan_error2() RETURNS text AS ' +plan = plpy.prepare("SELECT $1 AS testvalue1, $2 AS testvalue2", ["text", "text"]) +rv = plpy.execute(plan, u"\\x80", 1) +return rv[0]["testvalue1"] +' LANGUAGE plpythonu; diff --git a/src/pl/plpython/sql/plpython_schema.sql b/src/pl/plpython/sql/plpython_schema.sql index 2f8766431fa..1f5ee6eaea6 100644 --- a/src/pl/plpython/sql/plpython_schema.sql +++ b/src/pl/plpython/sql/plpython_schema.sql @@ -39,4 +39,6 @@ CREATE TABLE xsequences ( ) ; CREATE INDEX xsequences_pid_idx ON xsequences(pid) ; - +CREATE TABLE unicode_test ( + testvalue text NOT NULL +); -- GitLab