Re: libpq SSL with non-blocking sockets

Steve Singer <ssinger_pg@sympatico.ca>

From: Steve Singer <ssinger_pg@sympatico.ca>
To: Martin Pihlak <martin.pihlak@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, PG Hackers <pgsql-hackers@postgresql.org>
Date: 2011-06-24T21:14:39Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Improve libpq's error reporting for SSL failures.

  2. Use OpenSSL's SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER flag.

On 11-06-15 03:20 PM, Martin Pihlak wrote:
>
> Yes, that sounds like a good idea -- especially considering that COPY
> is not the only operation that can cause SSL_write retries.
>
>
> This is of course still "work in progress", needs cleaning up and
> definitely more testing. But at this point before going any further,
> I'd really appreciate a quick review from resident libpq gurus.
>

Martin,

I'm not a libpq guru but I've given your patch a look over.

The idea of storing the original buffer in new members of connection 
structure for later re-use seems like a good way to approach this.

A few things I noticed (that you might be aware of since you mentioned 
it needs cleanup)

-The patch doesn't compile with non-ssl builds,  the debug at the bottom 
of PQSendSome isn't in an #ifdef

-I don't think your handling the return code properly.   Consider this case.

pqSendSome(some data)
   sslRetryBuf = some Data
   return 1
pqSendSome(more data)
   it sends all of 'some data'
   returns 0

I think 1 should be returned because all of 'more data' still needs to 
be sent.  I think returning a 0 will break PQsetnonblocking if you call 
it when there is data in both sslRetryBuf and outputBuffer.
We might even want to try sending the data in outputBuffer after we've 
sent all the data sitting in sslRetryBuf.


If you close the connection with an outstanding sslRetryBuf you need to 
free it.


Other than running your test program I didn't do any testing with this 
patch.

Steve

> regards,
> Martin
>
>
>