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

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

In response to

Browse pgsql-hackers by date

  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)