Subject: Re: libpq: Process buffered SSL read bytes to support records >8kB on async API

From: Jacob Champion <[email-masked]>
Date: Tue, 10 Sep 2024 11:49:38 -0700

On Sun, Sep 8, 2024 at 1:08 PM Lars Kanis <[email-masked]> wrote:
> I'm the maintainer of ruby-pg the ruby interface to the PostgreSQL
> database. This binding uses the asynchronous API of libpq by default to
> facilitate the ruby IO wait and scheduling mechanisms.
>
> This works well with the vanilla postgresql server, but it leads to
> starvation with other types of servers using the postgresql wire
> protocol 3. This is because the current functioning of the libpq async
> interface depends on a maximum size of SSL records of 8kB.

Thanks for the report! I wanted evidence that this wasn't a
ruby-pg-specific problem, so I set up a test case with
Python/psycopg2.

I was able to reproduce a hang when all of the following were true:
- psycopg2's async mode was enabled
- the client performs a PQconsumeInput/PQisBusy loop, waiting on
socket read events when the connection is busy (I used
psycopg2.extras.wait_select() for this)
- the server splits a large message over many large TLS records
- the server packs the final ReadyForQuery message into the same
record as the split message's final fragment

Gory details of the packet sizes, if it's helpful:
- max TLS record size is 12k, because it made the math easier
- server sends DataRow of 32006 bytes, followed by DataRow of 806
bytes, followed by CommandComplete/ReadyForQuery
- so there are three TLS records on the wire containing
  1) DataRow 1 fragment 1 (12k bytes)
  2) DataRow 1 fragment 2 (12k bytes)
  3) DataRow 1 fragment 3 (7430 bytes) + DataRow 2 (806 bytes)
       + CommandComplete + ReadyForQuery

> To fix this issue the attached patch calls pqReadData() repeatedly in
> PQconsumeInput() until there is no buffered SSL data left to be read.
> Another solution could be to process buffered SSL read bytes in
> PQisBusy() instead of PQconsumeInput() .

I agree that PQconsumeInput() needs to ensure that the transport
buffers are all drained. But I'm not sure this is a complete solution;
doesn't GSS have the same problem? And are there any other sites that
need to make the same guarantee before returning?

I need to switch away from this for a bit. Would you mind adding this
to the next Commitfest as a placeholder?

    https://commitfest.postgresql.org/50/

Thanks,
--Jacob

---

From: Lars Kanis <[email-masked]>
Date: Wed, 11 Sep 2024 21:08:45 +0200

Thank you Jacob for verifying this issue!

>         + CommandComplete + ReadyForQuery

How did you verify the issue on the server side - with YugabyteDB or 
with a modified Postgres server? I'd like to verify the GSSAPI part and 
I'm familiar with the Postgres server only.

> need to make the same guarantee before returning?

Which other sites do you mean? The synchronous transfer already works, 
since the select() is short-circuit in case of pending bytes: [1]

> to the next Commitfest as a placeholder?

No problem; registered: https://commitfest.postgresql.org/50/5251/

--

Regards, Lars

[1] 
https://github.com/postgres/postgres/blob/77761ee5dddc0518235a51c533893e81e5f375b9/src/interfaces/libpq/fe-misc.c#L1070

---

From: Jacob Champion <[email-masked]>
Date: Wed, 11 Sep 2024 16:00:25 -0700

> I'm familiar with the Postgres server only.

Neither, unfortunately -- I have a protocol testbed that I use for
this kind of stuff. I'm happy to share once I get it cleaned up, but
it's unlikely to help you in this case since I haven't implemented
gssenc support. Patching the Postgres server itself seems like a good
way to go.

> > And are there any other sites that
> Which other sites do you mean?

I'm mostly worried that other parts of libpq might assume that a
single call to pqReadData will drain the buffers. If not, great! --
but I haven't had time to check all the call sites.

> No problem; registered: https://commitfest.postgresql.org/50/5251/

Thank you!

--Jacob

---

From: Heikki Linnakangas <[email-masked]>
Date: Fri, 3 Jul 2026 12:25:10 +0300

On 14/05/2026 07:32, solai v wrote:
> Hi,
> i tested the latest V3 patch series on current postgreSQL HEAD with SSL 
> enabled.The patches applied cleanly,build succeeded,and the server 
> started successfully.I ran the async libpq test repeatedly and also 
> tested large COPY TO STDOUT operations over SSL.I did not observe 
> hangs,crashes,or visible regressions during testing.

Thanks for the testing!

Here's another patch version, with some small comment and commit message 
changes, and I split the changes slightly differently between the two 
patches. End result is otherwise the same.

I plan to commit this in the next few days.

This question still remains:

> In pqReadData we have this:
> 
> 
>> 	/*
>> 	 * A return value of 0 could mean just that no data is now available, or
>> 	 * it could mean EOF --- that is, the server has closed the connection.
>> 	 * Since we have the socket in nonblock mode, the only way to tell the
>> 	 * difference is to see if select() is saying that the file is ready.
>> 	 * Grumble.  Fortunately, we don't expect this path to be taken much,
>> 	 * since in normal practice we should not be trying to read data unless
>> 	 * the file selected for reading already.
>> 	 *
>> 	 * In SSL mode it's even worse: SSL_read() could say WANT_READ and then
>> 	 * data could arrive before we make the pqReadReady() test, but the second
>> 	 * SSL_read() could still say WANT_READ because the data received was not
>> 	 * a complete SSL record.  So we must play dumb and assume there is more
>> 	 * data, relying on the SSL layer to detect true EOF.
>> 	 */
>> 
>> #ifdef USE_SSL
>> 	if (conn->ssl_in_use)
>> 		return 0;
>> #endif
> 
> 
> Should we do the same for GSS as we do for SSL here?

If someone more familiar with GSS would take a look at that, that'd be 
great. I assume that it has the same issue, and we should do the same 
for GSS as for SSL here. That's additional to these patches, though, so 
it doesn't prevent committing these now.

- Heikki
From fcfebe04c828a5f1ecfb52795cc63fcec7064908 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email-masked]>
Date: Fri, 3 Jul 2026 12:07:18 +0300
Subject: [PATCH v4 1/2] libpq: Extend "read pending" check from SSL to GSS

An extra check for pending bytes in the SSL layer has been part of
pqReadReady() for a very long time (79ff2e96d). But when GSS transport
encryption was added, it didn't receive the same treatment. (As
79ff2e96d notes, "The bug that I fixed in this patch is exceptionally
hard to reproduce reliably.")

Without that check, it's possible to hit a hang in gssencmode, if the
server splits a large libpq message such that the final message in a
streamed response is part of the same wrapped token as the split
message:

    DataRowDataRowDataRowDataRowDataRowData
    -- token boundary --
    RowDataRowCommandCompleteReadyForQuery

If the split message takes up enough memory to nearly fill libpq's
receive buffer, libpq may return from pqReadData() before the later
messages are pulled out of the PqGSSRecvBuffer. Without additional
socket activity from the server, pqReadReady() (via pqSocketCheck())
will never again return true, hanging the connection.

Pull the pending-bytes check into the pqsecure API layer, where both
SSL and GSS now implement it.

Note that this does not fix the root problem! Third party clients of
libpq have no way to call pqsecure_read_is_pending() in their own
polling. This just brings the GSS implementation up to par with the
existing SSL workaround; a broader fix is left to a subsequent commit.

In preparation for the broader fix, this patch already changes the
*_read_pending() functions to return the number of bytes in the buffer
rather than just a boolean. The current callers don't need that, but
the subsequent fix will.

Author: Jacob Champion <[email-masked]>
Discussion: https://postgr.es/m/CAOYmi%2BmpymrgZ76Jre2dx_PwRniS9YZojwH0rZnTuiGHCsj0rA%40mail.gmail.com
Backpatch-through: 14
---
 src/interfaces/libpq/fe-misc.c           |  6 ++---
 src/interfaces/libpq/fe-secure-gssapi.c  |  7 +++++
 src/interfaces/libpq/fe-secure-openssl.c | 34 +++++++++++++++++++++---
 src/interfaces/libpq/fe-secure.c         | 22 +++++++++++++++
 src/interfaces/libpq/libpq-int.h         |  6 +++--
 5 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 905344d5c38..58ccca1e393 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1099,14 +1099,12 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, pg_usec_time_t end_time)
 			return -1;
 		}
 
-#ifdef USE_SSL
-		/* Check for SSL library buffering read bytes */
-		if (forRead && conn->ssl_in_use && pgtls_read_pending(conn))
+		/* Check for SSL/GSS library buffering read bytes */
+		if (forRead && pqsecure_bytes_pending(conn) != 0)
 		{
 			/* short-circuit the select */
 			return 1;
 		}
-#endif
 	}
 
 	/* We will retry as long as we get EINTR */
diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
index 72f438dfa9c..cc60240582d 100644
--- a/src/interfaces/libpq/fe-secure-gssapi.c
+++ b/src/interfaces/libpq/fe-secure-gssapi.c
@@ -471,6 +471,13 @@ gss_read(PGconn *conn, void *recv_buffer, size_t length, ssize_t *ret)
 	return PGRES_POLLING_OK;
 }
 
+ssize_t
+pg_GSS_bytes_pending(PGconn *conn)
+{
+	Assert(PqGSSResultLength >= PqGSSResultNext);
+	return (ssize_t) (PqGSSResultLength - PqGSSResultNext);
+}
+
 /*
  * Negotiate GSSAPI transport for a connection.  When complete, returns
  * PGRES_POLLING_OK.  Will return PGRES_POLLING_READING or
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 6b44eeb68eb..1b22ba7b7f6 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -230,10 +230,38 @@ rloop:
 	return n;
 }
 
-bool
-pgtls_read_pending(PGconn *conn)
+ssize_t
+pgtls_bytes_pending(PGconn *conn)
 {
-	return SSL_pending(conn->ssl) > 0;
+	int			pending;
+
+	/*
+	 * OpenSSL readahead is documented to break SSL_pending().
+	 */
+	Assert(!SSL_get_read_ahead(conn->ssl));
+
+	pending = SSL_pending(conn->ssl);
+	if (pending < 0)
+	{
+		/* shouldn't be possible */
+		Assert(false);
+		libpq_append_conn_error(conn, "OpenSSL reports negative bytes pending");
+		return -1;
+	}
+	else if (pending == INT_MAX)
+	{
+		/*
+		 * If we ever found a legitimate way to hit this, we'd need to loop
+		 * around in the caller to call pgtls_bytes_pending() again.  Throw an
+		 * error rather than complicate the code in that way, because
+		 * SSL_read() should be bounded to the size of a single TLS record,
+		 * and conn->inBuffer can't currently go past INT_MAX in size anyway.
+		 */
+		libpq_append_conn_error(conn, "OpenSSL reports INT_MAX bytes pending");
+		return -1;
+	}
+
+	return (ssize_t) pending;
 }
 
 ssize_t
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 31d5b48d3f9..907cdb9ea39 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -243,6 +243,28 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
 	return n;
 }
 
+/*
+ *	Return the number of bytes available in the transport buffer.
+ *
+ * If pqsecure_read() is called for this number of bytes, it's guaranteed to
+ * return successfully without reading from the underlying socket.
+ */
+ssize_t
+pqsecure_bytes_pending(PGconn *conn)
+{
+#ifdef USE_SSL
+	if (conn->ssl_in_use)
+		return pgtls_bytes_pending(conn);
+#endif
+#ifdef ENABLE_GSS
+	if (conn->gssenc)
+		return pg_GSS_bytes_pending(conn);
+#endif
+
+	/* Plaintext connections have no transport buffer. */
+	return 0;
+}
+
 /*
  *	Write data to a secure connection.
  *
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 461b39620c3..3f921207a14 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -827,6 +827,7 @@ extern int	pqWriteReady(PGconn *conn);
 extern PostgresPollingStatusType pqsecure_open_client(PGconn *);
 extern void pqsecure_close(PGconn *);
 extern ssize_t pqsecure_read(PGconn *, void *ptr, size_t len);
+extern ssize_t pqsecure_bytes_pending(PGconn *);
 extern ssize_t pqsecure_write(PGconn *, const void *ptr, size_t len);
 extern ssize_t pqsecure_raw_read(PGconn *, void *ptr, size_t len);
 extern ssize_t pqsecure_raw_write(PGconn *, const void *ptr, size_t len);
@@ -863,9 +864,9 @@ extern void pgtls_close(PGconn *conn);
 extern ssize_t pgtls_read(PGconn *conn, void *ptr, size_t len);
 
 /*
- *	Is there unread data waiting in the SSL read buffer?
+ *	Return the number of bytes available in the transport buffer.
  */
-extern bool pgtls_read_pending(PGconn *conn);
+extern ssize_t pgtls_bytes_pending(PGconn *conn);
 
 /*
  *	Write data to a secure connection.
@@ -913,6 +914,7 @@ extern PostgresPollingStatusType pqsecure_open_gss(PGconn *conn);
  */
 extern ssize_t pg_GSS_write(PGconn *conn, const void *ptr, size_t len);
 extern ssize_t pg_GSS_read(PGconn *conn, void *ptr, size_t len);
+extern ssize_t pg_GSS_bytes_pending(PGconn *conn);
 #endif
 
 /* === in fe-trace.c === */
-- 
2.47.3

From 8383a00e7a2fe1f131416fb63e5e34fedf658243 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email-masked]>
Date: Fri, 3 Jul 2026 12:20:26 +0300
Subject: [PATCH v4 2/2] libpq: Drain all pending bytes from SSL/GSS during
 pqReadData()

The previous commit strengthened a workaround for a hang when large
messages are split across TLS records/GSS tokens. Because that
workaround is implemented in libpq internals, it can only help us when
libpq itself is polling on the socket. In nonblocking situations,
where the client above libpq is expected to poll, the same bugs can
show up.

As a contrived example, consider a large protocol-2.0 error coming
back from a server during PQconnectPoll(), split in an odd way across
two records:

    -- TLS record (8192-byte payload) --
    EEEE[...repeated a total of 8192 times]
    -- TLS record (8193-byte payload) --
    EEEE[...repeated a total of 8192 times]\0

The first record will fill the first half of the libpq receive buffer,
which is 16k long by default. The second record completely fills the
last half with its first 8192 bytes, leaving the terminating NULL in
the OpenSSL buffer. Since we still haven't seen the terminator at our
level, PQconnectPoll() will return PGRES_POLLING_READING, expecting to
come back when the server has sent "the rest" of the data.  But there
is nothing left to read from the socket; OpenSSL had to pull all of
the data in the 8193-byte record off of the wire to decrypt it.

A real server would probably not split up the records this way, nor
keep the connection open after sending a fatal connection error. But
servers that regularly use larger TLS records can get the libpq
receive buffer into the same state if DataRows are big enough, as
reported on the list. While the PostgreSQL server doesn't use larger
TLS records like that, other non-PostgreSQL servers that implement the
wire protocol are known to do that, as well as proxies that sit
between the server and the client

This is a layering violation. libpq makes decisions based on data in
the application buffer, above the transport buffer (whether SSL or
GSS), but clients are polling the socket below the transport buffer.
One way to fix this in a backportable way, without changing APIs too
much, is to ensure data never stays in the transport buffer. Then
pqReadData's postconditions will look similar for both raw sockets and
SSL/GSS: any available data is either in the application buffer, or
still on the socket.

Building on the prior commit, make pqReadData() to drain all pending
data from the transport layer into conn->inBuffer, expanding the
buffer as necessary. This is not particularly efficient from an
architectural perspective (the pqsecure_read() implementations take
care to fit their packets into the current buffer, and that effort is
now completely discarded), but it's hopefully easier to reason about
than a full rewrite would be for the back branches.

Author: Jacob Champion <[email-masked]>
Reviewed-by: Mark Dilger <[email-masked]>
Reviewed-by: solai v <[email-masked]>
Reported-by: Lars Kanis <[email-masked]>
Discussion: https://postgr.es/m/2039ac58-d3e0-434b-ac1a-2a987f3b4cb1%40greiz-reinsdorf.de
Backpatch-through: 14
---
 src/interfaces/libpq/fe-misc.c           | 145 ++++++++++++++++++++++-
 src/interfaces/libpq/fe-secure-openssl.c |   4 +-
 src/interfaces/libpq/fe-secure.c         |   3 +-
 3 files changed, 148 insertions(+), 4 deletions(-)

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 58ccca1e393..dd772838005 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -55,6 +55,8 @@ static int	pqPutMsgBytes(const void *buf, size_t len, PGconn *conn);
 static int	pqSendSome(PGconn *conn, int len);
 static int	pqSocketCheck(PGconn *conn, int forRead, int forWrite,
 						  pg_usec_time_t end_time);
+static int	pqReadData_internal(PGconn *conn);
+static int	pqDrainPending(PGconn *conn);
 
 /*
  * PQlibVersion: return the libpq version number
@@ -593,6 +595,13 @@ pqPutMsgEnd(PGconn *conn)
 
 /* ----------
  * pqReadData: read more data, if any is available
+ *
+ * Upon a successful return, callers may assume that either 1) all available
+ * bytes have been consumed from the socket, or 2) the socket is still marked
+ * readable by the OS.  (In other words: after a successful pqReadData, it's
+ * safe to tell a client to poll for readable bytes on the socket without any
+ * further draining of the SSL/GSS transport buffers.)
+ *
  * Possible return values:
  *	 1: successfully loaded at least one more byte
  *	 0: no data is presently available, but no error detected
@@ -605,8 +614,7 @@ pqPutMsgEnd(PGconn *conn)
 int
 pqReadData(PGconn *conn)
 {
-	int			someread = 0;
-	int			nread;
+	int			available;
 
 	if (conn->sock == PGINVALID_SOCKET)
 	{
@@ -614,6 +622,40 @@ pqReadData(PGconn *conn)
 		return -1;
 	}
 
+	available = pqReadData_internal(conn);
+	if (available < 0)
+		return -1;
+	else if (available > 0)
+	{
+		/*
+		 * Make sure there are no bytes stuck in layers between conn->inBuffer
+		 * and the socket, to make it safe for clients to poll on PQsocket().
+		 */
+		if (pqDrainPending(conn))
+			return -1;
+	}
+	else
+	{
+		/*
+		 * If we're not returning any bytes from the underlying transport,
+		 * that must imply there aren't any in the transport buffer...
+		 */
+		Assert(pqsecure_bytes_pending(conn) == 0);
+	}
+
+	return available;
+}
+
+/*
+ * Workhorse for pqReadData().  It's kept separate from the pqDrainPending()
+ * logic to avoid adding to this function's goto complexity.
+ */
+static int
+pqReadData_internal(PGconn *conn)
+{
+	int			someread = 0;
+	int			nread;
+
 	/* Left-justify any data in the buffer to make room */
 	if (conn->inStart < conn->inEnd)
 	{
@@ -800,6 +842,105 @@ definitelyFailed:
 	return -1;
 }
 
+/*---
+ * Drain any transport data that is already buffered in userspace and add it
+ * to conn->inBuffer, enlarging inBuffer if necessary.  The drain fails if
+ * inBuffer cannot be made to hold all available transport data.
+ *
+ * We assume that the underlying secure transport implementation does not
+ * attempt to read any more data from the socket while draining the transport
+ * buffer.  After a successful return, pqsecure_bytes_pending() must be zero.
+ *
+ * This operation is necessary to prevent deadlock, due to a layering
+ * violation designed into our asynchronous client API: pqReadData() and all
+ * the parsing routines above it receive data from the SSL/GSS transport
+ * buffer, but clients poll on the raw PQsocket() handle.  So data can be
+ * "lost" in the intermediate layer if we don't take it out here.
+ *
+ * To illustrate what we're trying to prevent, say that the server is sending
+ * two messages at once in response to a query (Aaaa and Bb), the libpq buffer
+ * is five characters in size, and TLS records max out at three-character
+ * payloads.  Here's what would happen if pqReadData() didn't call
+ * pqDrainPending():
+ *
+ *   Client    libpq      SSL      Socket
+ *     |         |         |         |
+ *     |      [     ]    [   ]     [   ]    [1] Buffers are empty, client is
+ *     x --------------------------> |          polling on socket
+ *     |         |         |         |
+ *     |      [     ]    [   ]     [xxx]    [2] First record is received; poll
+ *     | <-------------------------- |          signals read-ready
+ *     |         |         |         |
+ *     x ---> [     ]    [   ]     [xxx]    [3] Client calls PQconsumeInput()
+ *     |         |         |         |
+ *     |      [     ] -> [   ]     [xxx]    [4] libpq calls pqReadData() to fill
+ *     |         |         |         |          the receive buffer
+ *     |      [     ]    [Aaa] <-- [   ]    [5] SSL pulls payload off the wire
+ *     |         |         |         |          and decrypts it
+ *     |      [Aaa  ] <- [   ]     [   ]    [6] pqsecure_read() takes all data
+ *     |         |         |         |
+ *     | <--- [Aaa  ]    [   ]     [   ]    [7] PQconsumeInput() returns with a
+ *     x --------------------------> |          partial message, PQisBusy() is
+ *     |         |         |         |          still true, client polls again
+ *     |      [Aaa  ]    [   ]     [xxx]    [8] Second record is received; poll
+ *     | <-------------------------- |          signals read-ready
+ *     |         |         |         |
+ *     x ---> [Aaa  ]    [   ]     [xxx]    [9] Client calls PQconsumeInput()
+ *     |         |         |         |
+ *     |      [Aaa  ] -> [   ]     [xxx]   [10] libpq calls pqReadData() to fill
+ *     |         |         |         |          the receive buffer
+ *     |      [Aaa  ]    [aBb] <-- [   ]   [11] SSL decrypts
+ *     |         |         |         |
+ *     |      [AaaaB] <- [b  ]     [   ]   [12] pqsecure_read() fills its
+ *     |         |         |         |          buffer, taking only two bytes
+ *     | <--- [AaaaB]    [b  ]     [   ]   [13] PQconsumeInput() returns with a
+ *     |         |         |         |          complete message buffered;
+ *     |         |         |         |          PQisBusy() is false
+ *     x ---> [AaaaB]    [b  ]     [   ]   [14] Client calls PQgetResult()
+ *     |         |         |         |
+ *     | <--- [B    ]    [b  ]     [   ]   [15] Aaaa is returned; PQisBusy() is
+ *     x --------------------------> |          true and client polls again
+ *     .         |         |         .
+ *     .      [B    ]    [b  ]       .     [16] No packets, and client hangs.
+ *     .         |         |         .
+ *
+ * The pqDrainPending() call fixes the above scenario at step [13].  Before
+ * returning to the Client, it first expands the libpq buffer and moves the
+ * remaining data from the SSL buffer to the libpq buffer.
+ *
+ * The function returns 0 on success and -1 on error.  Success means that
+ * there was no data pending or it was successfully drained to conn->inBuffer.
+ * On error, conn->errorMessage is set.
+ */
+static int
+pqDrainPending(PGconn *conn)
+{
+	ssize_t		bytes_pending;
+	ssize_t		nread;
+
+	bytes_pending = pqsecure_bytes_pending(conn);
+	if (bytes_pending <= 0)
+		return bytes_pending;
+
+	/* Expand the input buffer if necessary. */
+	if (pqCheckInBufferSpace(conn->inEnd + (size_t) bytes_pending, conn))
+		return -1;				/* errorMessage already set */
+
+	nread = pqsecure_read(conn, conn->inBuffer + conn->inEnd,
+						  bytes_pending);
+	conn->inEnd += nread;
+
+	/* When there are bytes pending, the read function is not supposed to fail */
+	if (nread != bytes_pending)
+	{
+		libpq_append_conn_error(conn,
+								"drained only %zu of %zd pending bytes in transport buffer",
+								nread, bytes_pending);
+		return -1;
+	}
+	return 0;
+}
+
 /*
  * pqSendSome: send data waiting in the output buffer.
  *
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 1b22ba7b7f6..f8b2184a1ce 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -236,7 +236,9 @@ pgtls_bytes_pending(PGconn *conn)
 	int			pending;
 
 	/*
-	 * OpenSSL readahead is documented to break SSL_pending().
+	 * OpenSSL readahead is documented to break SSL_pending().  Plus, we can't
+	 * afford to have OpenSSL take bytes off the socket without processing
+	 * them; that breaks the postconditions for pqsecure_drain_pending().
 	 */
 	Assert(!SSL_get_read_ahead(conn->ssl));
 
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 907cdb9ea39..70faf8b2fe0 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -247,7 +247,8 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
  *	Return the number of bytes available in the transport buffer.
  *
  * If pqsecure_read() is called for this number of bytes, it's guaranteed to
- * return successfully without reading from the underlying socket.
+ * return successfully without reading from the underlying socket.  See
+ * pqDrainPending() for a more complete discussion of the concepts involved.
  */
 ssize_t
 pqsecure_bytes_pending(PGconn *conn)
-- 
2.47.3

---

From: Heikki Linnakangas <[email-masked]>
Date: Tue, 7 Jul 2026 18:58:42 +0300

On 03/07/2026 12:25, Heikki Linnakangas wrote:
>> i tested the latest V3 patch series on current postgreSQL HEAD with 
>> SSL enabled.The patches applied cleanly,build succeeded,and the server 

Committed and backpatched to all supported versions. Thanks everyone!

- Heikki

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 8 Jul 2026 21:16:18 +0200

On 07.07.26 17:58, Heikki Linnakangas wrote:
>>> SSL enabled.The patches applied cleanly,build succeeded,and the 
>>> server started successfully.I ran the async libpq test repeatedly and 
>>> also tested large COPY TO STDOUT operations over SSL.I did not 
>>> observe hangs,crashes,or visible regressions during testing.
>>
>> Thanks for the testing!
>>
>> Here's another patch version, with some small comment and commit 
>> message changes, and I split the changes slightly differently between 
>> the two patches. End result is otherwise the same.
>>

In pqDrainPending(), there is

     nread = pqsecure_read(conn, conn->inBuffer + conn->inEnd,
                           bytes_pending);
     conn->inEnd += nread;

But pqsecure_read() can return -1 for error.  So adding that to 
conn->inEnd at that point seems wrong.

There is error handling in the following code, but this would still kind 
of corrupt the conn->inEnd value?

     /* When there are bytes pending, the read function is not supposed 
to fail */
     if (nread != bytes_pending)
     {
         libpq_append_conn_error(conn,
                                 "drained only %zu of %zd pending bytes 
in transport buffer",
                                 nread, bytes_pending);
         return -1;
     }

I'm not sure if that comment means that a -1 return cannot happen?  Or 
just that the whole error check should not happen?

Also note that the format placeholder for nread is wrong.  If you do get 
a -1, it will print some large unsigned value.

---

From: Heikki Linnakangas <[email-masked]>
Date: Thu, 9 Jul 2026 15:55:59 +0300

On 08/07/2026 22:16, Peter Eisentraut wrote:
> But pqsecure_read() can return -1 for error.  So adding that to conn- 
>  >inEnd at that point seems wrong.
> 

The comment in pgsecure_bytes_pending() says:

>  * return successfully without reading from the underlying socket.  See
>  * pqDrainPending() for a more complete discussion of the concepts involved.

so as long as pqsecure_read() and pqsecure_read_pending() honor that 
contract, pqsecure_read() should not return an error. I agree that's a 
bit sloppy though, we should still check the return value.

The intention was also that it cannot do a short read, but that wasn't 
quite clear from the comment either.

> a -1, it will print some large unsigned value.

Good catch.

Patch attached to clean up all that.

- Heikki
From be68e94f81d8bb830cfda6eee6a20a41ec335933 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email-masked]>
Date: Thu, 9 Jul 2026 15:51:30 +0300
Subject: [PATCH 1/1] libpq: Make error checks in the new buffer draining code
 more robust

Check explicitly for pqsecure_read() returning an error. It shouldn't
fail, and we would've caught it in the check for a short read, but
better to be explicit so that the error message is more informative.
We also shouldn't update 'inEnd' when the read fails, although that
too is just pro forma as we will bail out and close the connection on
error.

Reported-by: Peter Eisentraut <[email-masked]>
Discussion: https://www.postgresql.org/message-id/[email-masked]
---
 src/interfaces/libpq/fe-misc.c   | 11 ++++++++---
 src/interfaces/libpq/fe-secure.c |  5 +++--
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index dd772838005..f11b58bf9c7 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -928,13 +928,18 @@ pqDrainPending(PGconn *conn)
 
-	conn->inEnd += nread;
 
-	/* When there are bytes pending, the read function is not supposed to fail */
+	/*
+	 * When there are bytes pending, pqsecure_read() is not supposed to fail
+	 * or do a short read, but let's check anyway to be safe.
+	 */
+	if (nread < 0)
+		return -1;
+	conn->inEnd += nread;
 	if (nread != bytes_pending)
 	{
 		libpq_append_conn_error(conn,
-								"drained only %zu of %zd pending bytes in transport buffer",
+								"drained only %zd of %zd pending bytes in transport buffer",
 								nread, bytes_pending);
 		return -1;
 	}
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 70faf8b2fe0..1a8e2e6746e 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -247,8 +247,9 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
  *	Return the number of bytes available in the transport buffer.
  *
  * If pqsecure_read() is called for this number of bytes, it's guaranteed to
- * return successfully without reading from the underlying socket.  See
- * pqDrainPending() for a more complete discussion of the concepts involved.
+ * return successfully with the same number of bytes, without reading from the
+ * underlying socket.  See pqDrainPending() for a more complete discussion of
+ * the concepts involved.
  */
 ssize_t
 pqsecure_bytes_pending(PGconn *conn)
-- 
2.47.3

---

From: Daniel Gustafsson <[email-masked]>
Date: Thu, 9 Jul 2026 17:06:55 +0200

> On 7 Jul 2026, at 17:58, Heikki Linnakangas <[email-masked]> wrote:

> Committed and backpatched to all supported versions. Thanks everyone!

Sorry for coming to this thread late.  When compiling against a local LibreSSL
on macOS I get an error on INT_MAX missing:

../src/interfaces/libpq/fe-secure-openssl.c:253:22: error: use of undeclared identifier 'INT_MAX'
        else if (pending == INT_MAX)
                            ^
1 error generated.

AFAICS it's because openssl/conf.h includes openssl/ossl_typ.h which in turn
includes <limits.h> but the LibreSSL counterpart does not.  That being said, it
clearly works in the buildfarm where we have LibreSSL animals on OpenBSD so it
must get pulled in indirectly from somewhere else there.  This might be
isolated to macOS builds, but I don't have other systems with LibreSSL handy so
not sure.  My LibreSSL installation is a libressl_portable locally compiled
which further may explain differences?

Any objections to the attached to make compilation pass when building against
LibreSSL on macOS (and possibly elsewhere)?

--
Daniel Gustafsson

---

From: Heikki Linnakangas <[email-masked]>
Date: Thu, 9 Jul 2026 18:52:12 +0300

On 09/07/2026 15:55, Heikki Linnakangas wrote:
>>  >inEnd at that point seems wrong.
>>
>> There is error handling in the following code, but this would still 
>> kind of corrupt the conn->inEnd value?
>>
>>      /* When there are bytes pending, the read function is not 
>> supposed to fail */
>>                                  "drained only %zu of %zd pending 
>> bytes in transport buffer",
>>                                  nread, bytes_pending);
>>          return -1;
>>      }
>>
>> Also note that the format placeholder for nread is wrong.  If you do 
>> get a -1, it will print some large unsigned value.
> 
> Good catch.
> 

Committed that, and Daniel's #include fix for LibreSSL too.

- Heikki