Hello!

On Wed, Mar 22, 2023 at 08:38:52PM +0400, Roman Arutyunyan wrote:

> On Sat, Mar 11, 2023 at 02:24:49PM +0300, Maxim Dounin wrote:
> > # HG changeset patch
> > # User Maxim Dounin <mdounin@mdounin.ru>
> > # Date 1678533841 -10800
> > # Sat Mar 11 14:24:01 2023 +0300
> > # Node ID b97f2b983d1564d29280d03828503edca21a79ee
> > # Parent 8771d35d55d0a2b1cefaab04401d6f837f5a05a2
> > Mail: fixed handling of blocked client read events in proxy.
> >
> > When establishing a connection to the backend, nginx blocks reading
> > from the client with ngx_mail_proxy_block_read(). Previously, such
> > events were lost, and in some cases this resulted in connection hangs.
> >
> > Notably, this affected mail_imap_ssl.t on Windows, since the test
> > closes connections after requesting authentication, but without
> > waiting for any responses (so the connection close events might be
> > lost).
>
> The following patch alowed me to trigger the issue on my laptop.
> It slows down the auth http backend and switches event processing to select.
>
> diff --git a/mail_imap_ssl.t b/mail_imap_ssl.t
> --- a/mail_imap_ssl.t
> +++ b/mail_imap_ssl.t
> @@ -42,6 +42,7 @@ my $t = Test::Nginx->new()->has(qw/mail
> daemon off;
>
> events {
> + use select;
> }
>
> mail {
> @@ -104,6 +105,7 @@ http {
> server {
> listen 127.0.0.1:8080;
> server_name localhost;
> + limit_rate 100;
>
> location = /mail/auth {
> access_log auth.log test;
>
> > Fix is to post an event to read from the client after connecting to
> > the backend if there were blocked events.
> >
> > diff --git a/src/mail/ngx_mail_proxy_module.c b/src/mail/ngx_mail_proxy_module.c
> > --- a/src/mail/ngx_mail_proxy_module.c
> > +++ b/src/mail/ngx_mail_proxy_module.c
> > @@ -327,7 +327,9 @@ ngx_mail_proxy_pop3_handler(ngx_event_t
> > c->log->action = NULL;
> > ngx_log_error(NGX_LOG_INFO, c->log, 0, "client logged in");
> >
> > - if (s->buffer->pos < s->buffer->last) {
> > + if (s->buffer->pos < s->buffer->last
> > + || s->connection->read->ready)
> > + {
> > ngx_post_event(c->write, &ngx_posted_events);
> > }
> >
> > @@ -486,7 +488,9 @@ ngx_mail_proxy_imap_handler(ngx_event_t
> > c->log->action = NULL;
> > ngx_log_error(NGX_LOG_INFO, c->log, 0, "client logged in");
> >
> > - if (s->buffer->pos < s->buffer->last) {
> > + if (s->buffer->pos < s->buffer->last
> > + || s->connection->read->ready)
> > + {
> > ngx_post_event(c->write, &ngx_posted_events);
> > }
> >
> > @@ -821,7 +825,9 @@ ngx_mail_proxy_smtp_handler(ngx_event_t
> > c->log->action = NULL;
> > ngx_log_error(NGX_LOG_INFO, c->log, 0, "client logged in");
> >
> > - if (s->buffer->pos < s->buffer->last) {
> > + if (s->buffer->pos < s->buffer->last
> > + || s->connection->read->ready)
> > + {
> > ngx_post_event(c->write, &ngx_posted_events);
> > }
>
> Looks ok

Thanks for the review, pushed to http://mdounin.ru/hg/nginx.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel