public inbox for [email protected]
help / color / mirror / Atom feedFrom: Heikki Linnakangas <[email protected]>
To: Peter Eisentraut <[email protected]>
To: solai v <[email protected]>
To: Jacob Champion <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: Mark Dilger <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Lars Kanis <[email protected]>
Cc: [email protected]
Cc: Merlin Moncure <[email protected]>
Subject: Re: libpq: Process buffered SSL read bytes to support records >8kB on async API
Date: Thu, 9 Jul 2026 18:52:12 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<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>
<[email protected]>
<CAHgHdKseseJG0j_96xrOuP3LgubvGScvXsQyPCYWANnBxTL4tA@mail.gmail.com>
<[email protected]>
<CAF0whuf=-G3SwXSTWyZNO148qoaEmth71cM7XBuX4X+2PnQkPQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
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
view thread (10+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: libpq: Process buffered SSL read bytes to support records >8kB on async API
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox