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

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