Обсуждение: FE/BE protocol oddity
[Repost from July 1. Hasn't made it to the list yet.]
The description of the FE/BE protocol says:
| The postmaster uses this info and the contents of the pg_hba.conf file
| to determine what authentication method the frontend must use. The
| postmaster then responds with one of the following messages:
[...]
| If the frontend does not support the authentication method requested by
| the postmaster, then it should immediately close the connection.
However, libpq doesn't do that. Instead, it leaves the connection open
and returns CONNECTION_BAD to the client. The client would then
presumably call something like PQfinish(), which sends a Terminate message
and closes the connection. This happened to not confuse the <=7.1
postmasters because they were waiting for 4 bytes and treated the early
connection close appropriately.
On this occasion let me also point out that
pqPuts("X", conn);
is not the way to send a single byte 'X' to the server. I see the JDBC
driver goes through troubles to make the same mistake.
In current sources the backends do the authentication and use the pqcomm.c
functions for communication, but those aren't that happy about the early
connection close:
pq_recvbuf: unexpected EOF on client connection FATAL 1: Password authentication failed for user 'peter' pq_flush:
send()failed: Broken pipe
So I figured I would sneak in a check for connection close before reading
the authentication response in the server, but since the frontends seems
to be doing what they want I don't really know what to check for.
Should I fix libpq to follow the docs in this and on the server's end make
anything that's not either a connection close or a valid authentication
response as "unexpected EOF"? That way old clients would produce a bit of
noise in the server log.
Does anyone know how the ODBC and JDBC drivers handle this situation?
Btw., is recv(sock, x, 1, MSG_PEEK) == 0 an appropriate way to check for a
closed connection without reading anything?
--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter
Peter Eisentraut <peter_e@gmx.net> writes:
> However, libpq doesn't do that. Instead, it leaves the connection open
> and returns CONNECTION_BAD to the client. The client would then
> presumably call something like PQfinish(), which sends a Terminate message
> and closes the connection. This happened to not confuse the <=7.1
> postmasters because they were waiting for 4 bytes and treated the early
> connection close appropriately.
Good point. Probably, PQfinish should only send the X message if the
connection has gotten past the authentication stage. A separate but
also useful change would be to do immediate socket close on detecting
auth failure, before returning to the client application.
> On this occasion let me also point out that
> pqPuts("X", conn);
> is not the way to send a single byte 'X' to the server.
Huh? Oh, the trailing null byte. You're right, it should bepqPutnchar("X", 1, conn);
> So I figured I would sneak in a check for connection close before reading
> the authentication response in the server, but since the frontends seems
> to be doing what they want I don't really know what to check for.
Seems reasonable, with the understanding that we'll still generate the
silly log messages when talking to an old client. However...
> Btw., is recv(sock, x, 1, MSG_PEEK) == 0 an appropriate way to check for a
> closed connection without reading anything?
Seems a little risky as far as portability goes; is MSG_PEEK supported
on BeOS, Darwin, Cygwin, etc? Might be better to fix the backend libpq
routines to understand whether a connection-close event is expected or
not, and only emit a complaint to the log when it's not. Not sure how
far such a change would need to propagate though...
regards, tom lane
Tom Lane writes: > Good point. Probably, PQfinish should only send the X message if the > connection has gotten past the authentication stage. A separate but > also useful change would be to do immediate socket close on detecting > auth failure, before returning to the client application. Further investigation shows that all of this used to work correctly until the infamous async connection patch. The comment now reads: (fe-connect.c) /* * We used to close the socket at this point, but that makes it * awkward for those above us if they wish to remove thissocket from * their own records (an fd_set for example). We'll just have this * socket closed when PQfinish is called(which is compulsory even * after an error, since the connection structure must be freed). */ I guess there is sort of a point there. So I'm leaning towards adding a "startup complete" flag somewhere in PGconn and simply fix up closePGconn(). > > Btw., is recv(sock, x, 1, MSG_PEEK) == 0 an appropriate way to check for a > > closed connection without reading anything? > > Seems a little risky as far as portability goes; is MSG_PEEK supported > on BeOS, Darwin, Cygwin, etc? Darwin is FreeBSD, so yes. Cygwin says it supports recv() and MSG_PEEK is trivial from there. BeOS is going backrupt before the next release anyway. ;-) Seriously, in the worst case we'll get EINVAL. > Might be better to fix the backend libpq routines to understand > whether a connection-close event is expected or not, and only emit a > complaint to the log when it's not. Not sure how far such a change > would need to propagate though... Deep... -- Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter
Peter Eisentraut <peter_e@gmx.net> writes:
> I guess there is sort of a point there. So I'm leaning towards adding a
> "startup complete" flag somewhere in PGconn and simply fix up
> closePGconn().
I think you can use the conn->status field; you shouldn't need a new
flag, just test whether status is CONNECTION_OK or not.
> Seriously, in the worst case we'll get EINVAL.
So you'll just ignore an error? Okay, that'll probably work.
regards, tom lane
Tom Lane writes: > I think you can use the conn->status field; you shouldn't need a new > flag, just test whether status is CONNECTION_OK or not. You're right. I was confused by the comment /* Non-blocking mode only below here */ in the definition of ConnStatusType, thinking that the non-blocking mode was using some of the intermediate states. I'll change it momentarily. -- Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter