Re: Password leakage avoidance

On 1/6/24 15:10, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> The only code specific comments were Tom's above, which have been 
>> addressed. If there are no serious objections I plan to commit this 
>> relatively soon.
> 
> I had not actually read this patchset before, but now I have, and
> I have a few minor suggestions:

Many thanks!

> * The API comment for PQchangePassword should specify that encryption
> is done according to the server's password_encryption setting, and
> probably the SGML docs should too.  You shouldn't have to read the
> code to discover that.

Check

> * I don't especially care for the useless initializations of
> encrypted_password, fmtpw, and fmtuser.  In all those cases the
> initial NULL is immediately replaced by a valid value.  Probably
> the compiler will figure out that the initializations are useless,
> but human readers have to do so as well.  Moreover, I think this
> style is more bug-prone not less so, because if you ever change
> the logic in a way that causes some code paths to fail to set
> the variables, you won't get use-of-possibly-uninitialized-value
> warnings from the compiler.
> 
> * Perhaps move the declaration of "buf" to the inner block where
> it's actually used?

Makes sense -- fixed

> * This could be shortened to just "return res":
> 
> +                 if (!res)
> +                     return NULL;
> +                 else
> +                     return res;

Heh, apparently I needed more coffee at this point :-)

> * I'd make the SGML documentation a bit more explicit about the
> return value, say
> 
> +       Returns a <structname>PGresult</structname> pointer representing
> +       the result of the <literal>ALTER USER</literal> command, or
> +       a null pointer if the routine failed before issuing any command.

Fixed.

I also ran pgindent. I was kind of surprised/unhappy when it made me 
change this (which aligned the two var names):
8<------------
<tab><tab><tab><tab>PQExpBufferData<tab>buf;
<tab><tab><tab><tab>PGresult<tab><sp><sp><sp>*res;
8<------------

to this (which leaves the var names unaligned):
8<------------
<tab><tab><tab><tab>PQExpBufferData<sp>buf;
<tab><tab><tab><tab>PGresult<sp><sp><sp>*res;
8<------------

Anyway, the resulting adjustments attached.

-- 
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com