Fix oversight in getParamDescriptions(), and improve comments.

When getParamDescriptions was changed to handle out-of-memory better
by cribbing error recovery logic from getRowDescriptions/getAnotherTuple,
somebody omitted to copy the stanza about checking for excess data in
the message.  But you need to do that, since continue'ing out of the
switch in pqParseInput3 means no such check gets applied there anymore.
Noted while looking at Michael Paquier's patch that made yet another
copy of this advance_and_error logic.

(This whole business desperately needs refactoring, because I sure don't
want to see a dozen copies of this code, but that's where we seem to be
headed.  What's more, the "suspend parsing on EOF return" convention is a
holdover from protocol 2 and shouldn't exist at all in protocol 3, because
we don't process partial messages anymore.  But for now, just fix the
obvious bug.)

Also, fix some wrong/missing comments about what the API spec is
for these three functions.

This doesn't seem worthy of back-patching, even though it's a bug;
the case shouldn't ever arise in the field.

diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 43898a45e37e4ecf62486a31f30bfb6c41332fe2..3034773972aaf9d3f31512eae9cb25ed2f898311 100644 (file)
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -467,7 +467,7 @@ handleSyncLoss(PGconn *conn, char id, int msgLength)
  * command for a prepared statement) containing the attribute data.
  * Returns: 0 if processed message successfully, EOF to suspend parsing
  * (the latter case is not actually used currently).
- * In either case, conn->inStart has been advanced past the message.
+ * In the former case, conn->inStart has been advanced past the message.
  */
 static int
 getRowDescriptions(PGconn *conn, int msgLength)
@@ -641,6 +641,7 @@ advance_and_error:
  * parseInput subroutine to read a 't' (ParameterDescription) message.
  * We'll build a new PGresult structure containing the parameter data.
  * Returns: 0 if completed message, EOF if not enough data yet.
+ * In the former case, conn->inStart has been advanced past the message.
  *
  * Note that if we run out of data, we have to release the partially
  * constructed PGresult, and rebuild it again next time.  Fortunately,
@@ -650,9 +651,9 @@ static int
 getParamDescriptions(PGconn *conn, int msgLength)
 {
        PGresult   *result;
+       const char *errmsg = NULL;      /* means "out of memory", see below */
        int                     nparams;
        int                     i;
-       const char *errmsg = NULL;
 
        result = PQmakeEmptyPGresult(conn, PGRES_COMMAND_OK);
        if (!result)
@@ -684,6 +685,13 @@ getParamDescriptions(PGconn *conn, int msgLength)
                result->paramDescs[i].typid = typid;
        }
 
+       /* Sanity check that we absorbed all the data */
+       if (conn->inCursor != conn->inStart + 5 + msgLength)
+       {
+               errmsg = libpq_gettext("extraneous data in \"t\" message");
+               goto advance_and_error;
+       }
+
        /* Success! */
        conn->result = result;
 
@@ -721,6 +729,11 @@ advance_and_error:
        printfPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
        pqSaveErrorResult(conn);
 
+       /*
+        * Return zero to allow input parsing to continue.  Essentially, we've
+        * replaced the COMMAND_OK result with an error result, but since this
+        * doesn't affect the protocol state, it's fine.
+        */
        return 0;
 }
 
@@ -729,7 +742,7 @@ advance_and_error:
  * We fill rowbuf with column pointers and then call the row processor.
  * Returns: 0 if processed message successfully, EOF to suspend parsing
  * (the latter case is not actually used currently).
- * In either case, conn->inStart has been advanced past the message.
+ * In the former case, conn->inStart has been advanced past the message.
  */
 static int
 getAnotherTuple(PGconn *conn, int msgLength)