Re: libpq should append auth failures, not overwrite

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: libpq should append auth failures, not overwrite
Date: 2018-08-15 14:51:44
Message-ID: CA+TgmoY9zDQtzABoO3twNZgvui-93bC-tzLvuBa28Yb7werBnw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 9, 2018 at 11:44 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I noticed that, although most error reports during libpq's connection
> setup code append to conn->errorMessage, the ones in fe-auth.c and
> fe-auth-scram.c don't: they're all printfPQExpBuffer() not
> appendPQExpBuffer(). This seems wrong to me. It makes no difference
> in simple cases with a single target server, but as soon as you have
> multiple servers listed in "host", this coding makes it impossible
> to tell which server rejected your login.
>
> So I think we should basically s/printfPQExpBuffer/appendPQExpBuffer/g
> anywhere those files touch conn->errorMessage, allowing any problems
> with previous servers to be preserved in the eventually-reported message.
> I won't bother posting an actual patch for that right now, but has anyone
> got an objection?

I guess the question in my mind is what behavior is most useful for
users. There are, I'm sure, cases where it's useful to see all the
details you are proposing to print. But it also seems like there
could be a significant number of cases where it's really more than
anybody wants to know. For instance, if I try to connect
host=primary1,primary2,dr and primary1 is offline right now, because
we're using primary2, and if it happens that I mistype my password,
with your patch, I'll get an error saying that primary1 is down,
followed by an error about my password. I only care about the second
one. If I had typed my password correctly, I would have just gotten a
connection, and everything would have been fine. Telling me that
primary1 is down may make me (1) panic or (2) miss the real cause of
my problem.

So I'm not as convinced as you are that always displaying maximum
detail is really the right thing to do.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2018-08-15 14:58:54 Re: C99 compliance for src/port/snprintf.c
Previous Message Robert Haas 2018-08-15 14:40:40 Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.