From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Cc: andrew@dunslane.net
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Residual cleanups for tied objects in PL/Perl
Date: Mon, 17 Aug 2026 14:08:28 -0400
Message-ID: <822136.1786990108@sss.pgh.pa.us> (raw)
In-Reply-To: <821480.1786989484@sss.pgh.pa.us>
References: <569769.1786901901@sss.pgh.pa.us>
<CAB8bMivzt9CGJWM--i3V5MUSXL2FSK2b1DPiJYow2rd9y6pu9A@mail.gmail.com>
<579928.1786912017@sss.pgh.pa.us>
<CAB8bMisdLJVgRt5nvUfU=ivCG6V778x225m93fwKXcq2zFvA-w@mail.gmail.com>
<CAB8bMisM-qSPw2qVnyjTp862LVAKpfX33_y-H1MBGzjAfwAQcg@mail.gmail.com>
<CAB8bMivZFmWh9vvkF5HBT+Cy9y=nA2NbGhxQapGPUaZ-mNerSA@mail.gmail.com>
<756684.1786981296@sss.pgh.pa.us>
<761068.1786985394@sss.pgh.pa.us>
<CAB8bMit2sRiVWfgW-ms1KciwofjHnDi4T=_xK4Zk2KFm7YBG0w@mail.gmail.com>
<CAB8bMis+5m1S_VEuGsbGVL=KG+vA8mYXoPdGQQOthwp7QAQSAA@mail.gmail.com>
<821480.1786989484@sss.pgh.pa.us>
Looking a bit more at the issue of standardizing our
perl-array-scanning code, I noticed that av_len returns SSize_t
not int, creating an overflow risk since we aren't consistent
about assigning the result to something wider than int. This
would only result in truncating the output not anything more
exciting, but it's still bad. Also, it seems from looking at
the Perl sources that they prefer to use av_count() to compute
the loop limit. That's the same as av_len()+1 (except it returns
Size_t), but I think that's a better habit to be in since it
removes the risk of forgetting to add 1. So here is a patch
that just cleans up those issues.
I'm inclined to push this, and separately push a patch to
clean up the inconsistent hash accessing logic, and then deal
with magic objects on top of that.
regards, tom lane
Attachments:
[text/x-diff] v2-0001-Make-plperl-s-handling-of-Perl-arrays-safer-and-m.patch (4.9K, ../822136.1786990108@sss.pgh.pa.us/2-v2-0001-Make-plperl-s-handling-of-Perl-arrays-safer-and-m.patch)
download | inline diff:
From 51d0d098b82276657fcc4e8d80c87ba3cf514f98 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 17 Aug 2026 13:37:33 -0400
Subject: [PATCH v2] Make plperl's handling of Perl arrays safer and more
consistent.
plperl_func_handler()'s stanza for handling an arrayref result in
a SETOF function could loop forever (or at least till OOM) when
given a tied array, since av_fetch won't necessarily ever return
a null pointer in that case. Be consistent with the other places
where we traverse a perl array: call av_len() once and use len+1
as the loop limit, silently ignoring any null pointers we get back
from that range of subscripts.
But actually, Perl's preferred locution for this seems to be to
use av_count() not av_len()+1. av_count() seems better since
there's less risk of forgetting to add 1. Also, both of those
functions return Size_t (or SSize_t) not int, creating at least
a theoretical overflow hazard. While we're modernizing this,
let's use the correct variable type where we can, and include an
overflow check where we can't.
Reported-by: Claude Code (via Noah Misch)
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us
Backpatch-through: 14
---
contrib/jsonb_plperl/jsonb_plperl.c | 5 ++--
src/pl/plperl/plperl.c | 41 ++++++++++++++++++++++-------
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/contrib/jsonb_plperl/jsonb_plperl.c b/contrib/jsonb_plperl/jsonb_plperl.cindex 97d147cc65a..00a99d303c6 100644--- a/contrib/jsonb_plperl/jsonb_plperl.c+++ b/contrib/jsonb_plperl/jsonb_plperl.c@@ -135,12 +135,11 @@ static void
AV_to_JsonbValue(AV *in, JsonbInState *jsonb_state)
{
dTHX;
- SSize_t pcount = av_len(in) + 1;- SSize_t i;+ Size_t pcount = av_count(in);
pushJsonbValue(jsonb_state, WJB_BEGIN_ARRAY, NULL);
- for (i = 0; i < pcount; i++)+ for (Size_t i = 0; i < pcount; i++)
{
SV **value = av_fetch(in, i, FALSE);
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.cindex eba91f2d7d6..8175407849e 100644--- a/src/pl/plperl/plperl.c+++ b/src/pl/plperl/plperl.c@@ -279,6 +279,7 @@ static void array_to_datum_internal(AV *av, ArrayBuildState **astatep,
int *ndims, int *dims, int cur_depth,
Oid elemtypid, int32 typmod,
FmgrInfo *finfo, Oid typioparam);
+static int av_count_limit(AV *av);
static Datum plperl_hash_to_datum(SV *src, TupleDesc td);
static void plperl_init_shared_libs(pTHX);
@@ -1173,8 +1174,8 @@ get_perl_array_ref(SV *sv)
* is frozen).
*
* Caller is required to have set dims[cur_depth - 1] to the length of the
- * input array, i.e., av_len(av) + 1. We make this requirement so as to- * avoid reading av_len() twice, which is hazardous for tied arrays.+ * input array, i.e., av_count_limit(av). We make this requirement so as to+ * avoid reading av_count() twice, which is hazardous for tied arrays.
*/
static void
array_to_datum_internal(AV *av, ArrayBuildState **astatep,
@@ -1214,11 +1215,11 @@ array_to_datum_internal(AV *av, ArrayBuildState **astatep,
errmsg("number of array dimensions exceeds the maximum allowed (%d)",
MAXDIM)));
/* OK, add a dimension */
- dims[*ndims] = av_len(nav) + 1;+ dims[*ndims] = av_count_limit(nav);
(*ndims)++;
}
else if (cur_depth >= *ndims ||
- av_len(nav) + 1 != dims[cur_depth])+ av_count_limit(nav) != dims[cur_depth])
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
errmsg("multidimensional arrays must have array expressions with matching dimensions")));
@@ -1260,6 +1261,25 @@ array_to_datum_internal(AV *av, ArrayBuildState **astatep,
}
}
+/*+ * av_count returns Size_t, so at least in theory it could overrun INT_MAX.+ * As long as we have to check, let's throw error for anything above+ * MaxArraySize, which will surely fail later.+ */+static int+av_count_limit(AV *av)+{+ dTHX;+ Size_t cnt = av_count(av);++ if (cnt > MaxArraySize)+ ereport(ERROR,+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),+ errmsg("array size exceeds the maximum allowed (%zu)",+ MaxArraySize)));+ return (int) cnt;+}+
/*
* convert perl array ref to a datum
*/
@@ -1287,7 +1307,7 @@ plperl_array_to_datum(SV *src, Oid typid, int32 typmod)
_sv_to_datum_finfo(elemtypid, &finfo, &typioparam);
memset(dims, 0, sizeof(dims));
- dims[0] = av_len(nav) + 1;+ dims[0] = av_count_limit(nav);
array_to_datum_internal(nav, &astate,
&ndims, dims, 1,
@@ -2478,14 +2498,15 @@ plperl_func_handler(PG_FUNCTION_ARGS)
if (sav)
{
dTHX;
- int i = 0;- SV **svp = 0;
AV *rav = (AV *) SvRV(sav);
+ Size_t alen = av_count(rav);- while ((svp = av_fetch(rav, i, FALSE)) != NULL)+ for (Size_t i = 0; i < alen; i++)
{
- plperl_return_next_internal(*svp);- i++;+ SV **svp = av_fetch(rav, i, FALSE);++ if (svp)+ plperl_return_next_internal(*svp);
}
}
else if (SvOK(perlret))
--
2.52.0
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-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pl0h0yp1@gmail.com, andrew@dunslane.net, pgsql-hackers@lists.postgresql.org
Subject: Re: Residual cleanups for tied objects in PL/Perl
In-Reply-To: <822136.1786990108@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