Re: query_int wrongly parses zero-padded input

From: Marko Grujic <marko(dot)grujic(at)enterprisedb(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Manu <manuelreyesbravo(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: query_int wrongly parses zero-padded input
Date: 2026-10-19 09:05:22
Message-ID: CAOvwyF0z_YJU9XBq9HKi1uQpcuFxF4StV_y6bEBbhSzcXdetFw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> Would "endptr == nnn" be more direct here? That checks whether no
conversion occurred, as described in the quoted doc.

Agreed, that would be more direct, with one caveat (which
admittedly doesn't apply today),
and that is that the endptr-not-at-NUL-byte variant also safe-guards
against trailing-junk (which Tom mentioned).

Thanks,
Marko

On Mon, Oct 19, 2026 at 10:52 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:

>
>
> > On Oct 19, 2026, at 16:30, Marko Grujic <marko(dot)grujic(at)enterprisedb(dot)com>
> wrote:
> >
> > Fair point about the bare minus sign, and the man pages for strtol(3p)
> seem to provide a neat way to handle it:
> >
> > > If the subject sequence is empty or does not have the expected
> > > form, no conversion is performed; the value of nptr shall be
> > > stored in the object pointed to by endptr, provided that endptr
> is
> > > not a null pointer.
> >
> > So it seems like if pass the 2nd arg (endptr), and it turns out to be
> something other than the NUL byte,
> > the parsing failed and we should return the ERR code (attached v2 now
> does that).
> >
> > Thanks,
> > Marko
> >
> >
> > On Mon, Oct 19, 2026 at 7:39 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >
> >
> > > On Oct 18, 2026, at 10:54, Manu <manuelreyesbravo(at)gmail(dot)com> wrote:
> > >
> > > Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> > >> The thing I was wondering about was whether an implementation might
> stop
> > >> short of reading the whole digit string once it realizes it's reached
> > >> overflow. [...] that's moot since we check both for errno being set
> and
> > >> for an out-of-int32-range result, both of which should get tripped if
> the
> > >> library does quit early on overflow.
> > >
> > > Point taken on the method; reasoning from the code rather than the
> test, the
> > > two checks turn out to be load-bearing on different platforms, which
> is why
> > > neither alone would do. gettoken() caps the operand at 15 characters
> > > (nnn[16]), so the value handed to strtol() is at most ~10^15. Where
> long is
> > > 64-bit, that is far below LONG_MAX, so strtol() cannot overflow and
> errno is
> > > never set on this input: an out-of-int32 value is read in full -- no
> early
> > > stop to worry about -- and is rejected only by the explicit
> > > "(long) *val != lval" round-trip. Where long is 32-bit, an out-of-int32
> > > operand does overflow strtol(), and POSIX requires ERANGE, which the
> errno
> > > check catches. So the "quit early on overflow" case is confined to
> platforms
> > > with a 32-bit long, and ERANGE covers it there; on a 64-bit long it
> can't
> > > arise. Either way no trailing-junk check is needed.
> > >
> >
> > <v2-0001-intarray-Parse-query_int-integers-as-decimals.patch>
>
> Would "endptr == nnn" be more direct here? That checks whether no
> conversion occurred, as described in the quoted doc. The check in v2 also
> works because nnn cannot be empty, so this is just a preference.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Previous Message Chao Li 2026-10-19 08:51:41 Re: query_int wrongly parses zero-padded input