Thread: empty array can crash backend using int_array_enum from contrib.
Using the int_array_enum function from contrib/intagg I can crash the 8.0.2 backend when I pass it an empty array. fli=# select int_array_enum('{}'::int[]); server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. !> fli=# select * from version(); version ------------------------------------------------------------------------------------- PostgreSQL 8.0.2 on i686-pc-linux-gnu, compiled by GCC gcc (GCC) 3.3.3 (SuSE Linux) (1 row)
Ron Mayer <rm_pg@cheapcomplexdevices.com> writes: > Using the int_array_enum function from contrib/intagg I can crash the 8.0.2 backend when I pass it an empty array. Man, we've had a few problems with that thing, haven't we? I patched it along these lines: *** contrib/intagg/int_aggregate.c.orig Thu Apr 14 14:16:08 2005 --- contrib/intagg/int_aggregate.c Sat Apr 23 01:32:52 2005 *************** *** 242,247 **** --- 242,250 ---- pc->p = p; pc->flags = 0; } + /* Now that we have a detoasted array, verify dimensions */ + if (pc->p->a.ndim != 1) + elog(ERROR, "int_enum only accepts 1-D arrays"); pc->num = 0; fcinfo->context = (Node *) pc; MemoryContextSwitchTo(oldcontext); regards, tom lane
On 2005-04-23, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Ron Mayer <rm_pg@cheapcomplexdevices.com> writes: >> Using the int_array_enum function from contrib/intagg I can crash the >> 8.0.2 backend when I pass it an empty array. > > Man, we've had a few problems with that thing, haven't we? > > I patched it along these lines: [snip] We were discussing this one on irc while it was presumably waiting in the moderation queue, and I suggested to the poster an alternative patch that allowed empty arrays to actually be treated as empty (your version will error out on int_array_enum('{}') rather than producing 0 rows, which seems unhelpful). I would suggest changing your test from != 1 to > 1, and adding the moral equivalent of: --- int_aggregate.c.orig Fri Apr 22 11:37:09 2005 +++ int_aggregate.c Fri Apr 22 11:44:34 2005 @@ -227,7 +227,7 @@ else /* use an existing one */ pc = (CTX *) fcinfo->context; /* Are we done yet? */ - if (pc->num >= pc->p->items) + if (ARR_NDIM(pc->p) != 1 || pc->num >= pc->p->items) { /* We are done */ if (pc->flags & TOASTED) (that test could be moved into the setup phase, of course) -- Andrew, Supernews http://www.supernews.com - individual and corporate NNTP services
Andrew - Supernews <andrew+nonews@supernews.com> writes: > We were discussing this one on irc while it was presumably waiting in the > moderation queue, and I suggested to the poster an alternative patch that > allowed empty arrays to actually be treated as empty (your version will > error out on int_array_enum('{}') rather than producing 0 rows, which seems > unhelpful). Done, but not back-patched since this seems more in the nature of a new feature than a crash preventative. regards, tom lane