Re: [BUG] hstore integer overflow when constructing large values

On Mon, Aug 17, 2026 at 10:15:15AM +0800, Tender Wang wrote:
> The reason I tried to keep `int32` for `buflen` in v1 was that
> `hstoreUniquePairs()` and `hstorePairs()` are exported functions. I was
> trying to keep the change minimally invasive in case the fix needed to be
> backpatched, and therefore avoided changing their argument types.
>
> Given that this does not seem worth backpatching, I agree that there is no
> good reason to preserve the existing `int32`-based size calculations.
> I've reworked the patch to use `Size` throughout the allocation-size
> calculation instead.

While looking at that, I am reaching similar conclusions in terms of
CALCDATASIZE(), and your version feels weird by having both a static
inline function *and* a macro.. Your previous hstoreAddPairLen() is
also tempting to keep. We apply the same rule in three places based
on if a pair is null or not.

> The former is the actual size of the hstore representation being passed to
> `palloc()`: 2160000003 bytes of key/value data plus 32 bytes for the hstore
> header and HEntry array. Thus the size calculation no longer overflows an
> `int32`, and the allocation limit is handled by the normal palloc machinery.

Attached is presumably what I would do, which has some similarities
with your v2, but it's a bit more expanded.

Please note some of the changes in hstore_compat.c, which happen due
to size_ being a uint32 but we decided to cast that to an int. I have
switched them while looking for patterns where a buflen was involved.
It does not seem reachable in practice, but as I'm looking at that
now, I may as well make that more consistent. Peter E. has done some
work that feels a bit familiar in e615da8cb21b, but these were under
the assumption that the end loop checks did not match with the
original ones. Here I'm changing both the counters and the end check
from one thing to the other, consistent with size_.

By the way, putting the attached patch aside for a second, I have
scratching my head for a bit to find out why your scenario was
failing. But that was due to -DWRITE_READ_PARSE_PLAN_TREES
-DCOPY_PARSE_PLAN_TREES..
--
Michael