| 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 08:30:46 |
| Message-ID: | CAOvwyF1UzFo7Muvo7e72qvHvx_ZMLnpsKjDae0N-PCVgoWzBSA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
> >
> > regards,
> > Manu
>
> The issue was introduced by 7ccb6dc2d3e, which switched atoi() to
> strtol(). Since atoi() uses base 10, changing strtol() to use base 10
> restores the original decimal interpretation. That seems reasonable.
>
> The variable "nnn" may contain an optional leading "-" sign followed by
> digits "0-9", so with base 10, conversion should consume the whole operand
> if it contains at least one digit. Overflow is handled by the existing
> errno and "(long) *val != lval" checks. This part also seems fine.
>
> The only thing I see is the handling of a bare "-" sign. I get an error on
> my MacBook:
> ```
> evantest=# select '-'::query_int;
> LINE 1: select '-'::query_int;
> ^
> ERROR: syntax error
> LINE 1: select '-'::query_int;
> ^
>
> evantest=# select '1&-'::query_int;
> LINE 1: select '1&-'::query_int;
> ^
> ERROR: syntax error
> LINE 1: select '1&-'::query_int;
> ^
> ```
>
> But this relies on strtol() setting errno. Reading strtol(3p) at [1],
> which is the POSIX Programmer's Manual, the relevant wording is:
> ```
> Upon successful completion, these functions shall return the
> converted value, if any. If no conversion could be performed, 0
> shall be returned and errno may be set to [EINVAL].
> ```
>
> It says 0 SHALL be returned, but errno MAY be set to EINVAL, which seems
> to imply that implementations are not required to set errno for a bare "-",
> which explains the difference between my result and Manu’s. I confirmed in
> the debugger that strtol("-") sets errno to EINVAL on my MacBook.
>
> So, based on the POSIX specification, we would need an end-pointer check
> to ensure that a bare "-" consistently raises an error.
>
> [1] https://man7.org/linux/man-pages/man3/strtol.3p.html
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
>
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-intarray-Parse-query_int-integers-as-decimals.patch | application/octet-stream | 2.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-10-19 08:49:09 | Re: DOCS - CREATE PUBLICATION wording re 'ONLY' for partitioned tables. |
| Previous Message | Peter Eisentraut | 2026-10-19 08:24:58 | Re: Add returns_nonnull to infallible allocators |