Thread: postgres 9.0 beta libpq empty binary array error
Hi, I'm using libpq PQexecParams ability to send query parameters as binary dat= a,=20 more specific - a binary representation of an empty array. I have the problem:=20 the code sending empty binary array works on 8.3 and 8.4=20 but stopped working on postgres 9.0 beta2/3/4, it fails with '22003','integ= er out of range error'. Here is the test code: char *command=3D"INSERT INTO test.arraytypes(nullarr) VALUES($1)"; // ndims hassnull typeid ((dim+lbound)*ndims) const int empty_array_length =3D 4 + 4 + 4 + (8*1); // constructing empty array representation: char * buf =3D (char *)malloc(empty_array_length); memset(buf,0,empty_array_length); char * out =3D buf; // ndims *((int*)out) =3D htonl(1); out+=3D4; // hassnull *((int*)out) =3D 0; out+=3D4; // typeid 'int4' *((int*)out) =3D htonl(23); out+=3D4; const Oid oids[] =3D {1007};// _int4 oid const int paramFormats[]=3D{1}; const int lens[] =3D {empty_array_length}; const char * const * vals =3D {&buf}; PGresult* re =3D PQexecParams(conn, command, 1, oids, (const char *const * ) vals, lens, paramFormats, 1); char *err1=3DPQresultErrorMessage(re); // ERROR: integer out of range for = 9.0 beta // sql creation code: //CREATE TABLE test.arraytypes( nullarr int[]) I've traced the error down to array_recv function, and found this overflow = check to be offending. /src/backend/utils/adt/arrayfuncs.c: Line 1214 for (i =3D 0; i < ndim; i++) { int ub; dim[i] =3D pq_getmsgint(buf, 4); lBound[i] =3D pq_getmsgint(buf, 4); ub =3D lBound[i] + dim[i] - 1; /* overflow? */ if (lBound[i] > ub) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); } In the empty array case, the ndim=3D=3D1, and dim[0]=3D=3D0, so lBound[0] >= ub[0] Seems that the overflow check was introduced fairly recently, here is the d= iscussion of it: http://archives.postgresql.org/pgsql-hackers/2009-08/msg02073.php The array with dim[i] =3D 0 seems legitimate, since this situation is handl= ed by the code below: if (nitems =3D=3D 0) { /* Return empty array ... but not till we've validated element_type */ PG_RETURN_ARRAYTYPE_P(construct_empty_array(element_type)); } So, is it really a bug in 9.0? Or maybe the array representation rules chan= ged somehow? Vladimir Shakhov | Developer www.alliedtesting.com
<vshahov@alliedtesting.com> writes: > I have the problem: > the code sending empty binary array works on 8.3 and 8.4 > but stopped working on postgres 9.0 beta2/3/4, it fails with '22003','integer out of range error'. I think you are right --- this rejects dim[i] == 0, but it should not. regards, tom lane
On 09/08/10 17:56, Tom Lane wrote: > <vshahov@alliedtesting.com> writes: >> I have the problem: >> the code sending empty binary array works on 8.3 and 8.4 >> but stopped working on postgres 9.0 beta2/3/4, it fails with '22003','integer out of range error'. > > I think you are right --- this rejects dim[i] == 0, but it should not. Yeah, this is also reproducible with COPY: postgres=# CREATE TABLE arrtest (a int4[]); CREATE TABLE postgres=# COPY ( SELECT array_fill(7, ARRAY[0], ARRAY[1]) ) TO '/tmp/a' BINARY; COPY 1 postgres=# COPY arrtest FROM '/tmp/a' BINARY;ERROR: integer out of range CONTEXT: COPY arrtest, line 1, column a The behavior of empty arrays with dimensions is weird in general. For example: postgres=# SELECT array_dims(array_fill(7, ARRAY[0, 1])); array_dims ------------ [1:0][1:1] (1 row) postgres=# SELECT array_fill(7, ARRAY[0, 1]); array_fill ------------ {} (1 row) postgres=# SELECT array_dims('{}'::int4[]); array_dims ------------ (1 row) The text representation of such an array is simply '{}', but that loses information about the dimensions. I agree that array_recv() should rather accept that than throw an error, patch attached, but this is pretty weird stuff... -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Attachment
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes: > The behavior of empty arrays with dimensions is weird in general. Agreed, but we shouldn't be introducing random restrictions in the name of security. Patch looks good to me, except that it occurs to me to wonder about negative values of dim[i]. For small negative values this coding will catch it, but what if it's large enough to overflow the other way? Maybe use if (dim[i] < 0 || lBound[i] > ub) ereport... regards, tom lane
On 09/08/10 21:26, Tom Lane wrote: > Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> writes: >> The behavior of empty arrays with dimensions is weird in general. > > Agreed, but we shouldn't be introducing random restrictions in the name > of security. > > Patch looks good to me, except that it occurs to me to wonder about > negative values of dim[i]. For small negative values this coding > will catch it, but what if it's large enough to overflow the other way? > Maybe use > > if (dim[i]< 0 || lBound[i]> ub) > ereport... ArrayGetNItems checks for dim[i] < 0. I concur though that it looks weird to not check that along with the overflow check, so maybe we should, just to make the code clearer. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On 09/08/10 21:29, Heikki Linnakangas wrote: > On 09/08/10 21:26, Tom Lane wrote: >> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> writes: >>> The behavior of empty arrays with dimensions is weird in general. >> >> Agreed, but we shouldn't be introducing random restrictions in the name >> of security. >> >> Patch looks good to me, except that it occurs to me to wonder about >> negative values of dim[i]. For small negative values this coding >> will catch it, but what if it's large enough to overflow the other way? >> Maybe use >> >> if (dim[i]< 0 || lBound[i]> ub) >> ereport... > > ArrayGetNItems checks for dim[i] < 0. I concur though that it looks > weird to not check that along with the overflow check, so maybe we > should, just to make the code clearer. Committed. I added a comment noting that dim[i] < 0 is checked in ArrayGetNItems. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com