I wrote: > Andrew Dunstan <andrew.duns...@2ndquadrant.com> writes: >> Patch proposed by Christoph Berg is here: >> https://www.postgresql.org/message-id/20190228151336.GB7550%40msg.df7cb.de
> Meh. That doesn't silence only the zero-bytes case, and I'm also > rather afraid of the fact that it's changing COMMERROR to something > else. I wonder whether (if client_min_messages <= DEBUG1) it could > result in trying to send the error message to the already-lost > connection. It might be that that can't happen, but I think a fair > amount of rather subtle (and breakable) analysis may be needed. Concretely, what about doing the following instead? This doesn't provide any mechanism for the DBA to adjust the logging behavior; but reducing log_min_messages to DEBUG1 would not be a very pleasant way to monitor for zero-data connections either, so I'm not that fussed about just dropping the message period for that case. I kind of like that we no longer need the weird special case for SSLdone. regards, tom lane
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index ccea231..fe59963 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1899,17 +1899,34 @@ ProcessStartupPacket(Port *port, bool SSLdone) MemoryContext oldcontext; pq_startmsgread(); - if (pq_getbytes((char *) &len, 4) == EOF) + + /* + * Grab the first byte of the length word separately, so that we can tell + * whether we have no data at all or an incomplete packet. (This might + * sound inefficient, but it's not really, because of buffering in + * pqcomm.c.) + */ + if (pq_getbytes((char *) &len, 1) == EOF) { /* - * EOF after SSLdone probably means the client didn't like our - * response to NEGOTIATE_SSL_CODE. That's not an error condition, so - * don't clutter the log with a complaint. + * If we get no data at all, don't clutter the log with a complaint; + * such cases often occur for legitimate reasons. An example is that + * we might be here after responding to NEGOTIATE_SSL_CODE, and if the + * client didn't like our response, it'll probably just drop the + * connection. Service-monitoring software also often just opens and + * closes a connection without sending anything. (So do port + * scanners, which may be less benign, but it's not really our job to + * notice those.) */ - if (!SSLdone) - ereport(COMMERROR, - (errcode(ERRCODE_PROTOCOL_VIOLATION), - errmsg("incomplete startup packet"))); + return STATUS_ERROR; + } + + if (pq_getbytes(((char *) &len) + 1, 3) == EOF) + { + /* Got a partial length word, so bleat about that */ + ereport(COMMERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("incomplete startup packet"))); return STATUS_ERROR; }