From: | Magnus Hagander <magnus(at)hagander(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: SSL-mode error reporting in libpq |
Date: | 2011-08-10 11:10:32 |
Message-ID: | CABUevEysYTJVCWw3PVnqGxN+RG_1A9JZxQuiK4ZzJfNynu=-TA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Jul 24, 2011 at 20:48, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> In testing the fix for the SSL problem that Martin Pihlak reported, I
> realized that libpq doesn't really cope very well with errors reported
> by OpenSSL. In the case at hand, SSL_write returns an SSL_ERROR_SSL
> code, which pqsecure_write quite reasonably handles by putting
> "SSL error: bad write retry" into conn->errorMessage. However, it
> then sets errno = ECONNRESET, which causes its caller pqSendSome()
> to overwrite that potentially-useful message with an outright lie:
> "server closed the connection unexpectedly".
>
> I think what we ought to do is adjust the code so that in SSL mode,
> pqsecure_write is responsible for constructing all error messages and
> pqSendSome should just leave conn->errorMessage alone.
>
> We could perhaps go a bit further and make pqsecure_write responsible
> for the error message in non-SSL mode too, but it looks to me like
> pqSendSome has to have a switch on the errno anyway to decide whether to
> keep trying or not, so moving that responsibility would just lead to
> duplicative coding.
>
> Any objections?
I haven't looked at the actual code but - does it make sense to have
them responsible for different parts and append them? If not, then no
objections ;)
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2011-08-10 12:08:14 | Re: longstanding mingw warning |
Previous Message | Magnus Hagander | 2011-08-10 10:53:44 | Re: Enforcing that all WAL has been replayed after restoring from backup |