Re: BUG #18907: SSL error: bad length failure during transfer data in pipeline mode with libpq - Mailing list pgsql-bugs

I wrote:
> Indeed, it looks like we'd better disable the round-down for GSSAPI
> too, because pg_GSS_write has exactly this same API requirement that
> caller has to pass at least as much data as last time.

So in short, I propose the attached patch.  I chose to disable the
round-down behavior for all TCP connections, including ones that
use neither SSL nor GSSAPI.  I'm not sure if it's worth worrying
too much about that case.

            regards, tom lane

diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index 3534f0b8111..5d98c58ffa8 100644
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -121,9 +121,9 @@ be_gssapi_write(Port *port, const void *ptr, size_t len)
      * again, so if it offers a len less than that, something is wrong.
      *
      * Note: it may seem attractive to report partial write completion once
-     * we've successfully sent any encrypted packets.  However, that can cause
-     * problems for callers; notably, pqPutMsgEnd's heuristic to send only
-     * full 8K blocks interacts badly with such a hack.  We won't save much,
+     * we've successfully sent any encrypted packets.  However, doing that
+     * expands the state space of this processing and has been responsible for
+     * bugs in the past (cf. commit d053a879b).  We won't save much,
      * typically, by letting callers discard data early, so don't risk it.
      */
     if (len < PqGSSSendConsumed)
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index c14e3c95250..dca44fdc5d2 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -553,9 +553,35 @@ pqPutMsgEnd(PGconn *conn)
     /* Make message eligible to send */
     conn->outCount = conn->outMsgEnd;

+    /* If appropriate, try to push out some data */
     if (conn->outCount >= 8192)
     {
-        int            toSend = conn->outCount - (conn->outCount % 8192);
+        int            toSend = conn->outCount;
+
+        /*
+         * On Unix-pipe connections, it seems profitable to prefer sending
+         * pipe-buffer-sized packets not randomly-sized ones, so retain the
+         * last partial-8K chunk in our buffer for now.  On TCP connections,
+         * the advantage of that is far less clear.  Moreover, it flat out
+         * isn't safe when using SSL or GSSAPI, because those code paths have
+         * API stipulations that if they fail to send all the data that was
+         * offered in the previous write attempt, we mustn't offer less data
+         * in this write attempt.  The previous write attempt might've been
+         * pqFlush attempting to send everything in the buffer, so we mustn't
+         * offer less now.  (Presently, we won't try to use SSL or GSSAPI on
+         * Unix connections, so those checks are just Asserts.  They'll have
+         * to become part of the regular if-test if we ever change that.)
+         */
+        if (conn->raddr.addr.ss_family == AF_UNIX)
+        {
+#ifdef USE_SSL
+            Assert(!conn->ssl_in_use);
+#endif
+#ifdef ENABLE_GSS
+            Assert(!conn->gssenc);
+#endif
+            toSend -= toSend % 8192;
+        }

         if (pqSendSome(conn, toSend) < 0)
             return EOF;
diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
index 62d05f68496..bc9e1ce06fa 100644
--- a/src/interfaces/libpq/fe-secure-gssapi.c
+++ b/src/interfaces/libpq/fe-secure-gssapi.c
@@ -112,9 +112,9 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
      * again, so if it offers a len less than that, something is wrong.
      *
      * Note: it may seem attractive to report partial write completion once
-     * we've successfully sent any encrypted packets.  However, that can cause
-     * problems for callers; notably, pqPutMsgEnd's heuristic to send only
-     * full 8K blocks interacts badly with such a hack.  We won't save much,
+     * we've successfully sent any encrypted packets.  However, doing that
+     * expands the state space of this processing and has been responsible for
+     * bugs in the past (cf. commit d053a879b).  We won't save much,
      * typically, by letting callers discard data early, so don't risk it.
      */
     if (len < PqGSSSendConsumed)