Message-ID: <1f35da9d-be6a-49df-9c56-e24c914c85a1@iki.fi>
Date: Thu, 9 Jul 2026 18:52:12 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: libpq: Process buffered SSL read bytes to support records >8kB on
 async API
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Peter Eisentraut <peter@eisentraut.org>, solai v <solai.cdac@gmail.com>,
 Jacob Champion <jacob.champion@enterprisedb.com>,
 Daniel Gustafsson <daniel@yesql.se>
Cc: Mark Dilger <mark.dilger@enterprisedb.com>,
 Andres Freund <andres@anarazel.de>, Lars Kanis <lars@greiz-reinsdorf.de>,
 pgsql-hackers@lists.postgresql.org, Merlin Moncure <mmoncure@gmail.com>
References: <2039ac58-d3e0-434b-ac1a-2a987f3b4cb1@greiz-reinsdorf.de>
 <CAOYmi+mpymrgZ76Jre2dx_PwRniS9YZojwH0rZnTuiGHCsj0rA@mail.gmail.com>
 <CAOYmi+=kbb7pV7k7RfWUcFt=CArJmp1R+R1Xnb8UnZLwpFgOwA@mail.gmail.com>
 <CAOYmi+==Y5ZZiwukN-wzGCyxdrL1dU3wOG-j0nza3MC3R2XhcA@mail.gmail.com>
 <kl6inl3iyycxqpiedmnnzaz2ytid5dw6knwwqk5plg6ej34oi6@fbyu4ll7ebzn>
 <CAOYmi+kZhv_LCg=g8Z6xtiueSYdz1fCvrXwAJ-i9m3VMm9_tXw@mail.gmail.com>
 <CAOYmi+mD6EbDEYfwZON0FCUAvGO+2=jR2V4KQYx+d+g0ap0Amg@mail.gmail.com>
 <CAOYmi+=YJ2J_sQ6yS8y1o9AX1Xgz35_r0hApV8QRWwi1K8gC0g@mail.gmail.com>
 <d987a763-09a1-4c7e-844e-bf8596abde43@iki.fi>
 <CAHgHdKseseJG0j_96xrOuP3LgubvGScvXsQyPCYWANnBxTL4tA@mail.gmail.com>
 <57d1e8b1-d016-4cea-9b60-63cbdb40eb81@iki.fi>
 <CAF0whuf=-G3SwXSTWyZNO148qoaEmth71cM7XBuX4X+2PnQkPQ@mail.gmail.com>
 <990a4710-fed1-411a-aba4-ddc25825ee48@iki.fi>
 <be2f229a-25e8-4d19-a307-e4ce3312f212@iki.fi>
 <34844e8c-267c-4daf-b1e0-f26059a4a7d3@eisentraut.org>
 <4cdfc0dc-9f66-4113-ad00-6e7a2cfa7211@iki.fi>
Content-Language: en-US
In-Reply-To: <4cdfc0dc-9f66-4113-ad00-6e7a2cfa7211@iki.fi>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/1f35da9d-be6a-49df-9c56-e24c914c85a1%40iki.fi>
Precedence: bulk

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