Make tsqueryout() use a StringInfo

While doing the recent security work on tsvector/tsquery overflows,
I wondered why tsqueryout() is using its very own hand-rolled
implementation of an extensible string buffer, rather than using
StringInfo like the rest of the backend. I couldn't see any actual
bug there, so changing it was out of scope for a security fix.
But it seems fragile and hard to read, so here's a patch to make it
use StringInfo.

regards, tom lane

Hi,

On Thu, 13 Aug 2026 at 21:16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> While doing the recent security work on tsvector/tsquery overflows,
> I wondered why tsqueryout() is using its very own hand-rolled
> implementation of an extensible string buffer, rather than using
> StringInfo like the rest of the backend. I couldn't see any actual
> bug there, so changing it was out of scope for a security fix.
> But it seems fragile and hard to read, so here's a patch to make it
> use StringInfo.
>

Thanks for the patch!

The patch looks good to me in general. One thing I wonder about is the
increase in the initial allocation: the old code starts with 32 bytes,
whereas initStringInfo() starts with 1024 bytes. ig this can add up when
tsqueryout() is called by array_out(), since array_out() retains each
element's output string while constructing the result?

Would it make sense to use initStringInfoExt(&nrm.buf, 32) here, preserving
the old initial size while retaining automatic growth?

Regards,
Ayush

Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> writes:
> The patch looks good to me in general. One thing I wonder about is the
> increase in the initial allocation: the old code starts with 32 bytes,
> whereas initStringInfo() starts with 1024 bytes. ig this can add up when
> tsqueryout() is called by array_out(), since array_out() retains each
> element's output string while constructing the result?

I kinda doubt that a huge array of tsquery's is a realistic scenario.

> Would it make sense to use initStringInfoExt(&nrm.buf, 32) here, preserving
> the old initial size while retaining automatic growth?

I don't think so. Maybe there's an argument that 1024 is too large,
but I would say that 32 is much too small. Also there are plenty of
other places using the default buffer length without worrying about
this. It seems unlikely to me that quibbling over the value is really
going to be a productive use of brain cells.

regards, tom lane

Hi,

On Sun, 16 Aug 2026 at 23:34, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> writes:
> > The patch looks good to me in general. One thing I wonder about is the
> > increase in the initial allocation: the old code starts with 32 bytes,
> > whereas initStringInfo() starts with 1024 bytes. ig this can add up when
> > tsqueryout() is called by array_out(), since array_out() retains each
> > element's output string while constructing the result?
>
> I kinda doubt that a huge array of tsquery's is a realistic scenario.
>
> > Would it make sense to use initStringInfoExt(&nrm.buf, 32) here,
> preserving
> > the old initial size while retaining automatic growth?
>
> I don't think so. Maybe there's an argument that 1024 is too large,
> but I would say that 32 is much too small. Also there are plenty of
> other places using the default buffer length without worrying about
> this. It seems unlikely to me that quibbling over the value is really
> going to be a productive use of brain cells.
>

Fair enough. I agree this probably isn't worth special-casing without a
realistic workload that demonstrates a problem.

It just seemed a decent bump hence called it out, the rest of the patch
looks good to me.

Regards,
Ayush