| From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
|---|---|
| To: | Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> |
| Cc: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Lars Kanis <lars(at)greiz-reinsdorf(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Merlin Moncure <mmoncure(at)gmail(dot)com> |
| Subject: | Re: libpq: Process buffered SSL read bytes to support records >8kB on async API |
| Date: | 2026-03-25 18:19:45 |
| Message-ID: | 57d1e8b1-d016-4cea-9b60-63cbdb40eb81@iki.fi |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 24/03/2026 18:44, Mark Dilger wrote:
> pqDrainPending() reads pending bytes into conn->inBuffer + conn->inEnd
> but never advances conn->inEnd. Compare with pqReadData_internal, which
> always does conn->inEnd += nread after a successful read. Is
> pgDrainPending()'s behavior correct? Sorry if I am misunderstanding.
> Without that update, could the drained bytes occupy buffer memory that
> libpq doesn't know about? Would the next call to pqReadData_internal
> overwrite those positions, silently losing the drained data?
You're right, that was broken, it indeed needs to advance conn->inEnd.
> In practice, I was unable to trigger this code path. When the protocol
> message parser in fe-protocol3.c (around line 122) sees a large DataRow
> header, it pre-expands the input buffer via pqCheckInBufferSpace() to
> hold the entire message. This ensures that the buffer always has enough
> free space for the next TLS record (max 16384 bytes plaintext), so
> SSL_read never returns a partial record and SSL_pending() is always 0 at
> the point pqDrainPending is called. Perhaps you thought of that, and
> that's why you didn't bother to worry about this?
>
> I suspect this is an unreachable bug, but perhaps it's just in need of
> some code comments. What are your thoughts?
It is reached whenever you hit the original bug. It is difficult to hit,
and you need something that sits between the PostgreSQL server and the
client because the server won't create large enough TLS fragments.
I wrote a little python script to reproduce this based on Jacob's
instructions at
https://www.postgresql.org/message-id/CAOYmi%2B%3DjZNU9mZ4Z83aR0RiqZ2pF35u5040gCxBgcFqoA4oNbQ%40mail.gmail.com.
See attached 'psycotest.py'. With the previous version, which didn't
advance conn->inEnd in pqDrainPending() it fails. With the attached
patch, where that's fixed, it succeeds.
- Heikki
| Attachment | Content-Type | Size |
|---|---|---|
| psycotest.py | text/x-python | 9.2 KB |
| v3-0001-libpq-Extend-read-pending-check-from-SSL-to-GSS.patch | text/x-patch | 5.7 KB |
| v3-0002-libpq-Drain-all-pending-bytes-from-SSL-GSS-during.patch | text/x-patch | 15.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2026-03-25 18:37:18 | Re: Better shared data structure management and resizable shared data structures |
| Previous Message | Tomas Vondra | 2026-03-25 18:02:02 | Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access) |