Re: Make formatting.c use StringInfos for output buffers

On 13/08/2026 20:24, Tom Lane wrote:
> We've had multiple security bugs (CVE-2026-14669, CVE-2015-0241) in
> formatting.c due to its habit of using output buffers of predetermined
> length, which are usually much too big but sometimes not big enough.
> And the path of least resistance for those security fixes was to
> impose arbitrary limits on substring lengths, which is surely a wart.
> I think it's time to stop the bleeding once and for all, by switching
> that code over to using StringInfos for its output buffers.
>
> That turns out to be a good deal more painful than one could wish,
> because the code is messy and inconsistent about how it uses its
> buffers; but detangling that seemed like a good code cleanup exercise
> anyway. So attached is a patch series to accomplish this:
>
> 0001: Convert DCH_to_char() to use a StringInfo output buffer.
>
> 0002: Restructure NUM_processor() to make it clearer which
> string is the output buffer.
>
> 0003: Convert NUM_processor() to use StringInfo output buffers.
>
> 0004: Try to buy back some performance (see below).

This all looks good to me.

> The main problem with this proposal is that it makes these functions
> a little slower, apparently because calling snprintf() via
> AppendStringInfo() is slower than calling it directly. After the
> performance hacking in 0004, what I see is that float8_to_char
> and numeric_to_number are the same speed or a little faster than
> before, but timestamptz_to_char is still around 10% slower in
> a tight-loop benchmark. (See drive_formatting.c, attached, for
> the benchmark infrastructure.) Maybe that's okay given that the
> overall effect on a complete SQL query should be far less, but
> I'd still like to squeeze out a bit more speed. I don't see any
> additional low-hanging fruit though.

More aggressive inlining seems like the straightforward solution. The
EMIT macros could do more...

You could replace the appendStringInfo() calls with more specialized
functions. Let's take "HH", for example. You could surely have a more
optimized implementation of that than calling appendStringInfo(out,
"%02d", ...).

With some inline functions and macros, you could probably still have it
read DCH_EMITF("%02d", ...) in the source code, but make the DCH_EMITF()
macro check that it's a compile-time constant and route it to a more
efficient function that gets fully inlined at compile time.

Attached is another micro-optimization that makes a surprisingly big
difference on my laptop (10% - 20%). In a nutshell, have a fast-path for
when a constant character in the format string is a one byte character.
I think it's a good bet that most constants in a format string are
characters like spaces, ":" or "-", which are a single ASCII character
in all locales.

I started with a slightly bigger refactoring to replace the
null-terminated FormatNode->character field with a separate length
field. That's also be pretty straightforward, but when I started to test
it, it turns out that you get the same effect from just the attached.

- Heikki