Re: [HACKERS] Incomplete startup packet errors

I wrote:
> Andrew Dunstan <andrew.dunstan@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;
     }