I wrote:
> Andrew Dunstan <[email protected]> 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;
}
