On 14.11.22 19:11, Jacob Champion wrote:
If we want to address this, maybe this should be handled
in the polling loop before we pass off the input buffer to the
per-message-type handlers.

I thought it was supposed to be handled by this code:

        /*
         * Can't process if message body isn't all here yet.
         */
        msgLength -= 4;
        avail = conn->inEnd - conn->inCursor;
        if (avail < msgLength)
        {
                /*
                 * Before returning, try to enlarge the input buffer if
                 * needed to hold the whole message; see notes in
                 * pqParseInput3.
                 */
                if (pqCheckInBufferSpace(conn->inCursor + (size_t) msgLength,
                                         conn))
                        goto error_return;
                /* We'll come back when there is more data */
                return PGRES_POLLING_READING;
        }

But after this block, we still treat EOF as if we need to get more data.
If we know that the message was supposed to be fully buffered, can we
just avoid the return to the pooling loop altogether and error out
whenever we see EOF?

I agree this doesn't make sense together. Digging through the history, this code is ancient and might have come about during the protocol 2/3 transition. (Protocol 2 didn't have length fields in the message IIRC.)

I think for the current code, the following would be an appropriate adjustment:

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 746e9b4f1efc..d15fb96572d9 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3412,8 +3412,7 @@ PQconnectPoll(PGconn *conn)
                /* Get the type of request. */
                if (pqGetInt((int *) &areq, 4, conn))
                {
-                   /* We'll come back when there are more data */
-                   return PGRES_POLLING_READING;
+                   goto error_return;
                }
                msgLength -= 4;

And then the handling of the 'v' message in my patch would also be adjusted like that.



Reply via email to