Re: Incorrect usage of strtol, atoi for non-numeric junk inputs

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: bharath(dot)rupireddyforpostgres(at)gmail(dot)com
Cc: alvherre(at)alvh(dot)no-ip(dot)org, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Incorrect usage of strtol, atoi for non-numeric junk inputs
Date: 2021-07-08 08:30:23
Message-ID: 20210708.173023.420899939748873659.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Wed, 7 Jul 2021 17:40:13 +0530, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote in
> On Fri, Jun 4, 2021 at 10:23 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> >
> > On 2021-Jun-04, Bharath Rupireddy wrote:
> >
> > > On Fri, Jun 4, 2021 at 8:58 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> >
> > > > I would suggest that the best way forward in this area is to rebase both
> > > > there patches on current master.
> > >
> > > Thanks. I will read both the threads [1], [2] and try to rebase the
> > > patches. If at all I get to rebase them, do you prefer the patches to
> > > be in this thread or in a new thread?
> >
> > Thanks, that would be helpful. This thread is a good place.
>
> I'm unable to spend time on this work as promised. I'd be happy if
> someone could take it forward, although it's not critical work(IMO)
> that needs immediate focus. I will try to spend time maybe later this
> year.

Looked through the three threads.

[1] is trying to expose pg_strtoint16/32 to frontend, but I don't see
much point in doing that in conjunction with [2] or this thread. Since
the integral parameter values of pg-commands are in int, which the
exising function strtoint() is sufficient to read. So even [2] itself
doesn't need to utilize [1].

So I agree to the Bharath's point.

So the attached is a standalone patch that:

- doesn't contain [1], since that functions are not needed for this
purpose.

- basically does the same thing with [2], but uses
strtoint/strtol/strtod instead of pg_strtoint16/32.

- doesn't try to make error messages verbose. That results in a
somewhat strange message like this but I'm not sure we should be so
strict at that point.

> reindexdb: error: number of parallel jobs must be at least 1: hoge

- is extended to cover all usages of atoi/l/f in command line
processing, which are not fully covered by [2]. (Maybe)

- is extended to cover psql's meta command parameters.

- is extended to cover integral environment variables. (PGPORTOLD/NEW
of pg_upgrade and COLUMNS of psql). The commands emit a warning for
invalid values, but I'm not sure it's worthwhile. (The second attached.)

> psql: warning: ignored invalid setting of environemt variable COLUMNS: 3x

- doesn't cover pgbench's meta command parameters (for speed).

[1] - https://www.postgresql.org/message-id/flat/alpine(dot)DEB(dot)2(dot)21(dot)1904201223040(dot)29102(at)lancre
[2] - https://www.postgresql.org/message-id/20191028012000.GA59064@begriffs.com

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
0001-Be-strict-in-numeric-parameters-on-command-line.patch text/x-patch 24.5 KB
0002-Make-complain-for-invalid-numeirc-values-in-environe.patch text/x-patch 2.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2021-07-08 08:47:11 Re: [PoC] Improve dead tuple storage for lazy vacuum
Previous Message Fabien COELHO 2021-07-08 08:26:23 Re: rand48 replacement