[PATCH] Mail: fixed handling of blocked client read events in proxy

[Roman Arutyunyan]

Hi,

On Sat, Mar 11, 2023 at 02:24:49PM +0300, Maxim Dounin wrote:
> # HG changeset patch
> # User Maxim Dounin <mdounin at 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