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

On Tue, Sep 10, 2024 at 11:49 AM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> I need to switch away from this for a bit.

"a bit"

In an effort to get this unstuck (and ideally solved during this
commitfest) here are my most recent thoughts:

> I agree that PQconsumeInput() needs to ensure that the transport
> buffers are all drained. But I'm not sure this is a complete solution;
> doesn't GSS have the same problem? And are there any other sites that
> need to make the same guarantee before returning?

So, Postgres internals cheat: calls to
pqWait/pqWaitTimed/pqSocketCheck() all reach down into the SSL buffer
to see if there are any pending bytes before polling the socket. I do
not yet understand why this protection is not extended to
GSS-encrypted connections.

In any case, our clients can't make use of that protection either, so
it's unfortunate that we're relying on it ourselves, because it makes
reasoning about a fix more difficult. I'm not about to propose
removing that case in a backport, but as long as it's there covering
up the problem, it's hard to say whether we've truly fixed this bug.

I think the hang could happen in any situation where a third-party
client calls into a function which calls pqReadData(), and then
immediately polls the socket for a read-ready condition. So: what are
the APIs that could be called that way?

Clearly PQconsumeInput() is one. PQconnectPoll() seems like it should
be exposed as well. postgres_fdw's use of PQgetResult() might qualify
(or if it doesn't, I don't understand why not -- there is at least one
path in PQgetResult() where the buffer is not fully consumed). And
nonblocking mode seems like it might sprinkle this hazard into more
places, as pqSendSome() will then start reading data and returning
before a pqWait().

So, to fix this once and for all, do we have to make sure that
pqReadData() always fully drains the SSL/GSS transport buffer instead
of eagerly returning?

--Jacob