Massmail problems with mail module

[mmgit@…]

We encountered a problem when sending mass-mails thru NGINX. When sending a mail to more than (in our case) ~130 recipients, NGINX closes the connection during the SMTP connection without an error.

What we found is, that this error only occurs when TLS and pipelining is involved. If either TLS or pipelining is deactivated, sending the mail works.
Another workaround is to increase the smtp_client_buffer. On our test system the mempage size is 4k.

What we saw in a capture is that when we send a mail with 130 recipients and pipelining is activated, NGINX receives the recipient commands and accepts them with 250 2.0.0 OK but then closes the TCP connection with a FIN.
I think it would be better that the sender gets an error if there is a to small buffer size.

[Maxim Dounin]

Thanks for the report, I think I see what is going on here.

When an SMTP command from the client cannot be read into the smtp_client_buffer buffer, nginx responds with "500 5.5.1 Invalid command" error and logs the "client sent too long command" error to the error log. But if the buffer is filled with pipelined SMTP commands and there is no free space in it, nginx might call recv() with zero length while trying to parse an additional command from the buffer. If this happens, recv() returns 0 and nginx thinks that the client closed the connection, so it closes the connection as well.

Please try the following patch, it should resolve this regardless of the smtp_client_buffer size (since nginx is only interested in one RCPT TO command before it passes the connection to the backend server, default buffer size of 4k is expected to be enough).

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1617811333 -10800
#      Wed Apr 07 19:02:13 2021 +0300
# Node ID 3d0102933b51fa7b9ca930442777a0e613a2ca8c
# Parent  ad5aa609ca30bb89f8b53f5a1d08bde5b964a3ac
Mail: fixed reading with fully filled buffer (ticket #2159).

With SMTP pipelining, ngx_mail_read_command() can be called with s->buffer
without any space available, to parse additional commands received to the
buffer on previous calls.  Previously, this resulted in recv() being
called with zero len, resulting in zero being returned, which was interpreted
as a connection close by the client, so nginx silently closed connection.

Fix is to avoid calling c->recv() if there is no free space in the buffer,
but continue parsing of the already received commands.

diff --git a/src/mail/ngx_mail_handler.c b/src/mail/ngx_mail_handler.c
--- a/src/mail/ngx_mail_handler.c
+++ b/src/mail/ngx_mail_handler.c
@@ -833,20 +833,23 @@ ngx_mail_read_command(ngx_mail_session_t
     ngx_str_t                  l;
     ngx_mail_core_srv_conf_t  *cscf;
 
-    n = c->recv(c, s->buffer->last, s->buffer->end - s->buffer->last);
+    if (s->buffer->last < s->buffer->end) {
+
+        n = c->recv(c, s->buffer->last, s->buffer->end - s->buffer->last);
 
-    if (n == NGX_ERROR || n == 0) {
-        ngx_mail_close_connection(c);
-        return NGX_ERROR;
-    }
+        if (n == NGX_ERROR || n == 0) {
+            ngx_mail_close_connection(c);
+            return NGX_ERROR;
+        }
 
-    if (n > 0) {
-        s->buffer->last += n;
-    }
+        if (n > 0) {
+            s->buffer->last += n;
+        }
 
-    if (n == NGX_AGAIN) {
-        if (s->buffer->pos == s->buffer->last) {
-            return NGX_AGAIN;
+        if (n == NGX_AGAIN) {
+            if (s->buffer->pos == s->buffer->last) {
+                return NGX_AGAIN;
+            }
         }
     }
 

Note that the patch is actually quite simple, though changes indentation of some code. Whitespace-agnostic version of the same patch is as follows:

diff --git a/src/mail/ngx_mail_handler.c b/src/mail/ngx_mail_handler.c
--- a/src/mail/ngx_mail_handler.c
+++ b/src/mail/ngx_mail_handler.c
@@ -833,6 +833,8 @@ ngx_mail_read_command(ngx_mail_session_t
     ngx_str_t                  l;
     ngx_mail_core_srv_conf_t  *cscf;
 
+    if (s->buffer->last < s->buffer->end) {
+
     n = c->recv(c, s->buffer->last, s->buffer->end - s->buffer->last);
 
     if (n == NGX_ERROR || n == 0) {
@@ -849,6 +851,7 @@ ngx_mail_read_command(ngx_mail_session_t
             return NGX_AGAIN;
         }
     }
+    }
 
     cscf = ngx_mail_get_module_srv_conf(s, ngx_mail_core_module);
 

[mmgit@…]

We will try the patch next week.
Thank you very much for the fast response.

[mmgit@…]

Tested the fix with 1.19.9 and it works.

[Maxim Dounin <mdounin@…>]

In 7829:2851e4c7de03/nginx:

Mail: fixed reading with fully filled buffer (ticket #2159).

With SMTP pipelining, ngx_mail_read_command() can be called with s->buffer
without any space available, to parse additional commands received to the
buffer on previous calls. Previously, this resulted in recv() being called
with zero length, resulting in zero being returned, which was interpreted
as a connection close by the client, so nginx silently closed connection.

Fix is to avoid calling c->recv() if there is no free space in the buffer,
but continue parsing of the already received commands.

[Maxim Dounin]

Thanks for testing, fix committed.