Thread: postgres 9.0 beta libpq empty binary array error

postgres 9.0 beta libpq empty binary array error

From
Date:
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

Re: postgres 9.0 beta libpq empty binary array error

From
Tom Lane
Date:
<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

Re: postgres 9.0 beta libpq empty binary array error

From
Heikki Linnakangas
Date:
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

Re: postgres 9.0 beta libpq empty binary array error

From
Tom Lane
Date:
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

Re: postgres 9.0 beta libpq empty binary array error

From
Heikki Linnakangas
Date:
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

Re: postgres 9.0 beta libpq empty binary array error

From
Heikki Linnakangas
Date:
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