On 09/07/2026 15:55, Heikki Linnakangas wrote:
On 08/07/2026 22:16, Peter Eisentraut wrote:
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?
The comment in pgsecure_bytes_pending() says:
* 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.
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.
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.
Patch attached to clean up all that.
Committed that, and Daniel's #include fix for LibreSSL too.
- Heikki
