From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17912: Invalid memory access when converting plpython' array containing empty array
Date: Fri, 28 Apr 2023 15:17:09 -0400
Message-ID: <1483429.1682709429@sss.pgh.pa.us> (raw)
In-Reply-To: <1440309.1682705916@sss.pgh.pa.us>
References: <17912-82ceed78731d9cdc@postgresql.org>
<1392974.1682694846@sss.pgh.pa.us>
<bf33fe9b-f2f8-5885-aae3-537cd4591386@gmail.com>
<1440309.1682705916@sss.pgh.pa.us>
I wrote:
> Yeah. AFAICT, the idea of the existing code is to descend through
> the first item at each nest level till we hit a non-list, then take
> the lengths of those lists as the array dimensions, and then complain
> if we find any later items that don't fit those dimensions. That leads
> to some symmetry problems, in that the error you get for inconsistent
> sequence lengths depends on the order in which the items are presented.
The real problem here is that we don't check that the list nesting
depth is the same throughout the array: if lists are more deeply
nested in later elements, we'll treat those sub-lists as scalars,
leading to inconsistent results. Conversely, a less-deeply-nested
list structure in a later element might still work, if it can be
treated as a sequence. I think the second and third examples
I gave should both throw errors.
I also notice that the error messages in this area speak of "sequences",
but it is more correct to call them "lists", because Python draws a
distinction. (All lists are sequences, but not vice versa, eg a
string is a sequence but not a list.)
So I'm thinking about the attached. I do not propose this for
back-patching, because it could break applications that work today.
But it seems like good tightening-up for HEAD, or maybe we should
wait for v17 at this point?
regards, tom lane
Attachments:tighten-multidim-array-shape-checks.patch (text/x-diff, 6.3K) downloaddiff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.outindex c4e749a5a8..f477e239bc 100644--- a/src/pl/plpython/expected/plpython_types.out+++ b/src/pl/plpython/expected/plpython_types.out@@ -700,10 +700,26 @@ CREATE FUNCTION test_type_conversion_mdarray_malformed() RETURNS int[] AS $$
return [[1,2,3],[4,5]]
$$ LANGUAGE plpython3u;
SELECT * FROM test_type_conversion_mdarray_malformed();
-ERROR: wrong length of inner sequence: has length 2, but 3 was expected-DETAIL: To construct a multidimensional array, the inner sequences must all have the same length.+ERROR: wrong length of inner list: has length 2, but 3 was expected+DETAIL: To construct a multidimensional array, the inner lists must all have the same length.
CONTEXT: while creating return value
PL/Python function "test_type_conversion_mdarray_malformed"
+CREATE FUNCTION test_type_conversion_mdarray_malformed2() RETURNS text[] AS $$+return [[1,2,3], "abc"]+$$ LANGUAGE plpython3u;+SELECT * FROM test_type_conversion_mdarray_malformed2();+ERROR: array element is a scalar, but should be a list+DETAIL: To construct a multidimensional array, the inner lists must be nested to a uniform depth.+CONTEXT: while creating return value+PL/Python function "test_type_conversion_mdarray_malformed2"+CREATE FUNCTION test_type_conversion_mdarray_malformed3() RETURNS text[] AS $$+return ["abc", [1,2,3]]+$$ LANGUAGE plpython3u;+SELECT * FROM test_type_conversion_mdarray_malformed3();+ERROR: array element is a list, but should be a scalar+DETAIL: To construct a multidimensional array, the inner lists must be nested to a uniform depth.+CONTEXT: while creating return value+PL/Python function "test_type_conversion_mdarray_malformed3"
CREATE FUNCTION test_type_conversion_mdarray_toodeep() RETURNS int[] AS $$
return [[[[[[[1]]]]]]]
$$ LANGUAGE plpython3u;
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.cindex 864b5f1765..eddedec3d4 100644--- a/src/pl/plpython/plpy_typeio.c+++ b/src/pl/plpython/plpy_typeio.c@@ -1126,7 +1126,7 @@ PLyObject_ToTransform(PLyObToDatum *arg, PyObject *plrv,
/*
- * Convert Python sequence to SQL array.+ * Convert Python sequence (or list of lists) to SQL array.
*/
static Datum
PLySequence_ToArray(PLyObToDatum *arg, PyObject *plrv,
@@ -1205,8 +1205,15 @@ PLySequence_ToArray(PLyObToDatum *arg, PyObject *plrv,
dims[0] = PySequence_Length(plrv);
}
- /* Allocate space for work arrays, after detecting array size overflow */
len = ArrayGetNItems(ndim, dims);
+ if (len == 0)+ {+ /* If no elements, ensure we return a zero-D array */+ array = construct_empty_array(arg->u.array.elmbasetype);+ return PointerGetDatum(array);+ }++ /* Allocate space for work arrays, after detecting array size overflow */
elems = palloc(sizeof(Datum) * len);
nulls = palloc(sizeof(bool) * len);
@@ -1246,35 +1253,55 @@ PLySequence_ToArray_recurse(PLyObToDatum *elm, PyObject *list,
{
int i;
- if (PySequence_Length(list) != dims[dim])- ereport(ERROR,- (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),- errmsg("wrong length of inner sequence: has length %d, but %d was expected",- (int) PySequence_Length(list), dims[dim]),- (errdetail("To construct a multidimensional array, the inner sequences must all have the same length."))));-- if (dim < ndim - 1)+ /*+ * The list nesting depth in all array items must match what+ * PLySequence_ToArray saw in the first item. At the first dimension, we+ * might have a non-list sequence, but recurse anyway.+ */+ if (dim == 0 || PyList_Check(list))
{
- for (i = 0; i < dims[dim]; i++)- {- PyObject *sublist = PySequence_GetItem(list, i);+ int thisdim = PySequence_Length(list);- PLySequence_ToArray_recurse(elm, sublist, dims, ndim, dim + 1,- elems, nulls, currelem);- Py_XDECREF(sublist);- }- }- else- {- for (i = 0; i < dims[dim]; i++)+ if (thisdim < 0)+ PLy_elog(ERROR, "could not determine sequence length for function return value");++ /* like PLySequence_ToArray, we treat 0-length list as a scalar */+ if (thisdim > 0)
{
- PyObject *obj = PySequence_GetItem(list, i);+ /* Check for uniform array structure */+ if (dim >= ndim)+ ereport(ERROR,+ (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),+ errmsg("array element is a list, but should be a scalar"),+ (errdetail("To construct a multidimensional array, the inner lists must be nested to a uniform depth."))));+ if (thisdim != dims[dim])+ ereport(ERROR,+ (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),+ errmsg("wrong length of inner list: has length %d, but %d was expected",+ thisdim, dims[dim]),+ (errdetail("To construct a multidimensional array, the inner lists must all have the same length."))));+ /* OK, so recurse */+ for (i = 0; i < thisdim; i++)+ {+ PyObject *sublist = PySequence_GetItem(list, i);- elems[*currelem] = elm->func(elm, obj, &nulls[*currelem], true);- Py_XDECREF(obj);- (*currelem)++;+ PLySequence_ToArray_recurse(elm, sublist, dims, ndim, dim + 1,+ elems, nulls, currelem);+ Py_XDECREF(sublist);+ }+ return;
}
}
++ /* The "list" is, or should be treated as, a scalar */+ if (dim != ndim)+ ereport(ERROR,+ (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),+ errmsg("array element is a scalar, but should be a list"),+ (errdetail("To construct a multidimensional array, the inner lists must be nested to a uniform depth."))));++ elems[*currelem] = elm->func(elm, list, &nulls[*currelem], true);+ (*currelem)++;
}
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sqlindex 9702a10a72..b57b9a6463 100644--- a/src/pl/plpython/sql/plpython_types.sql+++ b/src/pl/plpython/sql/plpython_types.sql@@ -341,6 +341,18 @@ $$ LANGUAGE plpython3u;
SELECT * FROM test_type_conversion_mdarray_malformed();
+CREATE FUNCTION test_type_conversion_mdarray_malformed2() RETURNS text[] AS $$+return [[1,2,3], "abc"]+$$ LANGUAGE plpython3u;++SELECT * FROM test_type_conversion_mdarray_malformed2();++CREATE FUNCTION test_type_conversion_mdarray_malformed3() RETURNS text[] AS $$+return ["abc", [1,2,3]]+$$ LANGUAGE plpython3u;++SELECT * FROM test_type_conversion_mdarray_malformed3();+
CREATE FUNCTION test_type_conversion_mdarray_toodeep() RETURNS int[] AS $$
return [[[[[[[1]]]]]]]
$$ LANGUAGE plpython3u;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17912: Invalid memory access when converting plpython' array containing empty array
In-Reply-To: <1483429.1682709429@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox