Re: PATCH SSL_pending() checks in libpq/fe-misc.c - Mailing list pgsql-hackers

Would you send over a context diff, diff -c?


---------------------------------------------------------------------------

Jack Bates wrote:
> 
> Hello:
> 
> I took a look at the SSL code in libpq/fe-misc.c and noticed what I 
> think is a small problem.  A patch is included at the bottom of this 
> email against anoncvs TopOfTree this evening.
> 
> The SSL library buffers input data internally.  Nowhere in libpq's code 
> is this buffer being checked via SSL_pending(), which can lead to a 
> condition where once in a while a socket appears to "hang" or "lag". 
>  This is because select() won't see bytes buffered by the library.  A 
> condition like this is most likely to occur when the library's read 
> buffer has been filled previously and another read is to be performed. 
>  If the end of the backend's transmission was less than one SSL frame 
> payload away from the last byte returned in the previous read, this will 
> likely hang.  Trust me that I learned of this most painfully...  
> 
> I am looking deeper at how to enable non-blocking SSL sockets in libpq. 
>  As Tom Lane states, this is primarily a matter of checking SSL error 
> codes, particularly for SSL_WANT_READ and SSL_WANT_WRITE, and reacting 
> appropriately.  I'll see about that as I have more free time.
> 
> Even though I'm doing this, I tend to agree with Tom that SSH tunnels 
> are a really good way to make the whole SSL problem just go away.
> 
> My quick patch to perform the SSL_pending() checks:
> 
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-misc.c,v
> retrieving revision 1.70
> diff -r1.70 fe-misc.c
> 350a351
>  >  * -or- if SSL is enabled and used, is it buffering bytes?
> 361a363,371
>  > /* Check for SSL library buffering read bytes */
>  > #ifdef USE_SSL
>  >       if (conn->ssl && SSL_pending(conn->ssl) > 0)
>  >       {
>  >               /* short-circuit the select */
>  >               return 1;
>  >       }
>  > #endif
>  >
> 784a795,797
>  >  * If SSL enabled and used and forRead, buffered bytes short-circuit the
>  >  * call to select().
>  >  *
> 801a815,823
>  >
>  > /* Check for SSL library buffering read bytes */
>  > #ifdef USE_SSL
>  >       if (forRead && conn->ssl && SSL_pending(conn->ssl) > 0)
>  >       {
>  >               /* short-circuit the select */
>  >               return 0;
>  >       }
>  > #endif
> 
> _Of_course_ I am just fine with this patch being under a Berkeley-style 
> license and included in PostgreSQL.
> 
> Cheers.
> 
> -- 
> 
> Jack Bates
> Portland, OR, USA
> http://www.floatingdoghead.net
> 
> Got privacy?
> My PGP key: http://www.floatingdoghead.net/pubkey.txt
> 
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026