Fwd: 1.17.5 regression

[Maxim Dounin]

Hello!

On Thu, Dec 19, 2019 at 09:01:10AM -0800, Maksim Yevmenkin wrote:

> hello,
> 
> [...]
> 
> > > > > Thanks for the report, it indeed looks like a bug introduced
> > > > > in 9d2ad2fb4423.
> > > > >
> > > > > The problem is that c->read->handler is overwritted when switching
> > > > > to the next pipelined request, ngx_ssl_next_read_handler() is not
> > > > > called, and c->read->ready remains not set.  I'll take a look how
> > > > > to fix it properly.
> > > >
> > > > Thanks for having a look.
> > > >
> > > > Please keep me updated when the fix gets applied.
> 
> [...]
> 
> > Not really.  I meant the workaround in question won't work on
> > systems with kqueue.  The problem itself is present with kqueue as
> > well.
> 
> i suspect there is another instance of the same problem. it manifests
> itself when nginx is reading reueqst over https, i.e. ngx_ssl_recv()
> is interacting with ngx_http_process_request_line() and friends.
> rev->handler gets overwritten, and, ngx_ssl_next_read_handler() never
> gets called.

Yes, any change of connection read handlers can be problematic 
here.  

> i wonder if SSL_pending(3) can be of value here, i.e.

No, SSL_pending() only returns pending bytes from the SSL record 
currently being parsed, and there can be additional records in SSL 
library buffers even if SSL_pending() returns 0.  Hence it is 
useless.

The only working alternative solution I'm aware of is to use 
BIO_set_callback() and count actual bytes read from the socket.  
Unfortunately, BoringSSL removed BIO_set_callback(), making this 
solution non-portable.

-- 
Maxim Dounin
http://mdounin.ru/