Fix bug in previous patch: following data is permitted after '.' so it
authorPhilip Hazel <ph10@hermes.cam.ac.uk>
Mon, 16 Apr 2007 10:31:58 +0000 (10:31 +0000)
committerPhilip Hazel <ph10@hermes.cam.ac.uk>
Mon, 16 Apr 2007 10:31:58 +0000 (10:31 +0000)
must not be diagnosed as an error. The check for vanished socket can
only be applied when there is no data pending.

15 files changed:
doc/doc-txt/ChangeLog
src/src/receive.c
test/log/0559
test/log/2029
test/log/2150
test/scripts/0000-Basic/0300
test/scripts/0000-Basic/0301
test/scripts/0000-Basic/0559
test/scripts/2000-GnuTLS/2029
test/scripts/2100-OpenSSL/2150
test/stdout/0300
test/stdout/0301
test/stdout/0559
test/stdout/2029
test/stdout/2150

index 4a8d41d..db68bda 100644 (file)
@@ -1,4 +1,4 @@
-$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.502 2007/04/13 15:13:47 ph10 Exp $
+$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.503 2007/04/16 10:31:58 ph10 Exp $
 
 Change log file for Exim from version 4.21
 -------------------------------------------
@@ -203,14 +203,16 @@ SC/03 Eximstats - V1.58 Fix to get <> and blackhole to show in edomain tables.
 PH/43 Yet another patch from the Sieve maintainer.
 
 PH/44 I found a way to check for a TCP/IP connection going away before sending
-      the response to the final '.' that terminates a message. At this time,
-      there should not be any pending input - the client should be waiting for
-      the response. Therefore, a select() can be used: if it shows that the
-      input is "ready", there is either input waiting, or the socket has been
-      closed. Both cases are errors. (It's a bit more complicated than this
-      because of buffering, but that's the essence of it.) Previously, Exim
+      the response to the final '.' that terminates a message, but only in the
+      case where the client has not sent further data following the '.'
+      (unfortunately, this is allowed). However, in many cases there won't be
+      any further data because there won't be any more messages to send. A call
+      to select() can be used: if it shows that the input is "ready", there is
+      either input waiting, or the socket has been closed. An attempt to read
+      the next input character can distinguish the two cases. Previously, Exim
       would have sent an OK response which the client would never have see.
-      This could lead to message repetition. This fix should cure that.
+      This could lead to message repetition. This fix should cure that, at
+      least in a lot of common cases.
 
 
 Exim version 4.66
index 98a728b..e4c82d2 100644 (file)
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/receive.c,v 1.36 2007/04/13 15:13:47 ph10 Exp $ */
+/* $Cambridge: exim/src/src/receive.c,v 1.37 2007/04/16 10:31:58 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -3436,21 +3436,26 @@ to cause a call to receive_bomb_out() if the log cannot be opened. */
 
 receive_call_bombout = TRUE;
 
-/* Before sending an SMTP response in a TCP/IP session, we check to see if
-there is unconsumed input (which there shouldn't be) or if the connection has
-gone away. This can be done because the end of a message is always a
-synchronization point. If the connection is still present, but there is no
-pending input, the result of a select() call will be zero. If, however, the
-connection has gone away, or if there is pending input, the result of select()
-will be non-zero. The two cases can be distinguished by trying to read the next
-input character. Of course, since TCP/IP is asynchronous, there is always a
-chance that the connection will vanish between the time of this test and the
-sending of the response, but the chance of this happening should be small.
+/* Before sending an SMTP response in a TCP/IP session, we check to see if the
+connection has gone away. This can only be done if there is no unconsumed input
+waiting in the local input buffer. We can test for this by calling
+receive_smtp_buffered(). RFC 2920 (pipelining) explicitly allows for additional
+input to be sent following the final dot, so the presence of following input is
+not an error.
 
-We also check for input that has already been received and is in the local
-input buffer (plain SMTP or TLS) by calling receive_smtp_buffered(). */
+If the connection is still present, but there is no unread input for the
+socket, the result of a select() call will be zero. If, however, the connection
+has gone away, or if there is pending input, the result of select() will be
+non-zero. The two cases can be distinguished by trying to read the next input
+character. If we succeed, we can unread it so that it remains in the local
+buffer for handling later. If not, the connection has been lost.
 
-if (smtp_input && sender_host_address != NULL && !sender_host_notsocket)
+Of course, since TCP/IP is asynchronous, there is always a chance that the
+connection will vanish between the time of this test and the sending of the
+response, but the chance of this happening should be small. */
+
+if (smtp_input && sender_host_address != NULL && !sender_host_notsocket &&
+    !receive_smtp_buffered())
   {
   struct timeval tv;
   fd_set select_check;
@@ -3459,47 +3464,39 @@ if (smtp_input && sender_host_address != NULL && !sender_host_notsocket)
   tv.tv_sec = 0;
   tv.tv_usec = 0;
 
-  if (select(fileno(smtp_in) + 1, &select_check, NULL, NULL, &tv) != 0 ||
-      receive_smtp_buffered())
+  if (select(fileno(smtp_in) + 1, &select_check, NULL, NULL, &tv) != 0)
     {
-    uschar *msg;
-    if ((RECEIVE_GETC)() == EOF)
-      {
-      msg = US"SMTP connection lost after final dot";
-      smtp_reply = US"";   /* No attempt to send a response */
-      }
-    else
+    int c = (RECEIVE_GETC)();
+    if (c != EOF) (RECEIVE_UNGETC)(c); else
       {
-      msg = US"Synchronization error (data after final dot)";
-      smtp_reply = US"550 Synchronization error (data after final dot)";
-      }
-
-    /* Overwrite the log line workspace */
+      uschar *msg = US"SMTP connection lost after final dot";
+      smtp_reply = US"";    /* No attempt to send a response */
+      smtp_yield = FALSE;   /* Nothing more on this connection */
 
-    sptr = 0;
-    s = string_cat(s, &size, &sptr, msg, Ustrlen(msg));
-    s = add_host_info_for_log(s, &size, &sptr);
-    s[sptr] = 0;
-    log_write(0, LOG_MAIN, "%s", s);
+      /* Re-use the log line workspace */
 
-    /* We now have to delete the files for this aborted message. */
+      sptr = 0;
+      s = string_cat(s, &size, &sptr, msg, Ustrlen(msg));
+      s = add_host_info_for_log(s, &size, &sptr);
+      s[sptr] = 0;
+      log_write(0, LOG_MAIN, "%s", s);
 
-    sprintf(CS spool_name, "%s/input/%s/%s-D", spool_directory, message_subdir,
-      message_id);
-    Uunlink(spool_name);
+      /* Delete the files for this aborted message. */
 
-    sprintf(CS spool_name, "%s/input/%s/%s-H", spool_directory, message_subdir,
-      message_id);
-    Uunlink(spool_name);
+      sprintf(CS spool_name, "%s/input/%s/%s-D", spool_directory,
+        message_subdir, message_id);
+      Uunlink(spool_name);
 
-    sprintf(CS spool_name, "%s/msglog/%s/%s", spool_directory, message_subdir,
-      message_id);
-    Uunlink(spool_name);
+      sprintf(CS spool_name, "%s/input/%s/%s-H", spool_directory,
+        message_subdir, message_id);
+      Uunlink(spool_name);
 
-    /* Do not accept any more messages on this connection. */
+      sprintf(CS spool_name, "%s/msglog/%s/%s", spool_directory,
+        message_subdir, message_id);
+      Uunlink(spool_name);
 
-    smtp_yield = FALSE;
-    goto TIDYUP;
+      goto TIDYUP;
+      }
     }
   }
 
index 077691e..fcae1cf 100644 (file)
@@ -1,4 +1,2 @@
 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
 1999-03-02 09:44:33 10HmaX-0005vi-00 SMTP connection lost after final dot H=(abcd) [127.0.0.1] P=esmtp
-1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
-1999-03-02 09:44:33 10HmaY-0005vi-00 Synchronization error (data after final dot) H=(abcd) [127.0.0.1] P=esmtp
index 737b697..0e16a7b 100644 (file)
@@ -1,5 +1,3 @@
 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
 1999-03-02 09:44:33 10HmaX-0005vi-00 TLS recv error on connection from [127.0.0.1]: A TLS packet with unexpected length was received.
 1999-03-02 09:44:33 10HmaX-0005vi-00 SMTP connection lost after final dot H=[127.0.0.1] P=smtps
-1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
-1999-03-02 09:44:33 10HmaY-0005vi-00 Synchronization error (data after final dot) H=[127.0.0.1] P=smtps
index 41ada44..01c4307 100644 (file)
@@ -1,4 +1,2 @@
 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
 1999-03-02 09:44:33 10HmaX-0005vi-00 SMTP connection lost after final dot H=[127.0.0.1] P=smtps
-1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
-1999-03-02 09:44:33 10HmaY-0005vi-00 Synchronization error (data after final dot) H=[127.0.0.1] P=smtps
index de3be10..053aec8 100644 (file)
@@ -23,9 +23,8 @@ rset\r\nmail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata
 ??? 250
 ??? 354
 the message
-.
+.\r\nmail from:<userx@test.ex>
 +++ 1
-mail from:<userx@test.ex>
 rcpt to:<userx@test.ex>\r\ndata\r\nthe message\r\nsecond line
 ??? 250
 ??? 250
index f84244f..4add8f4 100644 (file)
@@ -26,9 +26,8 @@ mail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata
 ??? 250
 ??? 354
 the message
-.
+.\r\nmail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata\r\nthe message
 ??? 250
-mail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata\r\nthe message
 ??? 250
 ??? 250
 ??? 354
index 01d7d99..3729f25 100644 (file)
@@ -20,25 +20,3 @@ This is a test message.
 ****
 sleep 1
 killdaemon
-#
-# Also check for next input sent too soon
-#
-exim -DSERVER=server -bd -oX PORT_D
-****
-client -t5 127.0.0.1 PORT_D
-??? 220
-ehlo abcd
-??? 250-
-??? 250-
-??? 250-
-??? 250
-mail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata
-??? 250
-??? 250
-??? 354
-This is a test message.
-.\r\nrset
-??? 550
-****
-sleep 1
-killdaemon
index e371e68..c2b3549 100644 (file)
@@ -28,30 +28,3 @@ This is a test message.
 ****
 sleep 1
 killdaemon
-#
-# Also check for next input sent too soon
-#
-exim -DSERVER=server -bd -oX PORT_D
-****
-client-gnutls 127.0.0.1 PORT_D
-??? 220
-ehlo abcd
-??? 250-
-??? 250-
-??? 250-
-??? 250-
-??? 250
-starttls
-??? 220
-mail from:<userx@test.ex>
-??? 250
-rcpt to:<userx@test.ex>
-??? 250
-data
-??? 354
-This is a test message.
-.\r\nrset
-+++ 1
-****
-sleep 1
-killdaemon
index cfc6a20..91cd382 100644 (file)
@@ -26,30 +26,3 @@ This is a test message.
 ****
 sleep 1
 killdaemon
-#
-# Also check for next input sent too soon
-#
-exim -DSERVER=server -bd -oX PORT_D
-****
-client-gnutls 127.0.0.1 PORT_D
-??? 220
-ehlo abcd
-??? 250-
-??? 250-
-??? 250-
-??? 250-
-??? 250
-starttls
-??? 220
-mail from:<userx@test.ex>
-??? 250
-rcpt to:<userx@test.ex>
-??? 250
-data
-??? 354
-This is a test message.
-.\r\nrset
-+++ 1
-****
-sleep 1
-killdaemon
index 8dd5a3c..a41df39 100644 (file)
@@ -27,9 +27,8 @@ Connecting to 127.0.0.1 port 1225 ... connected
 ??? 354
 <<< 354 Enter message, ending with "." on a line by itself
 >>> the message
->>> .
+>>> .\r\nmail from:<userx@test.ex>
 +++ 1
->>> mail from:<userx@test.ex>
 >>> rcpt to:<userx@test.ex>\r\ndata\r\nthe message\r\nsecond line
 ??? 250
 <<< 250 OK id=10HmaX-0005vi-00
index 964f0c1..198962d 100644 (file)
@@ -36,10 +36,9 @@ Connecting to 127.0.0.1 port 1225 ... connected
 ??? 354
 <<< 354 Enter message, ending with "." on a line by itself
 >>> the message
->>> .
+>>> .\r\nmail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata\r\nthe message
 ??? 250
 <<< 250 OK id=10HmaX-0005vi-00
->>> mail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata\r\nthe message
 ??? 250
 <<< 250 OK
 ??? 250
index eee1b2e..71f5c45 100644 (file)
@@ -21,27 +21,3 @@ Connecting to 127.0.0.1 port 1225 ... connected
 >>> .
 +++ 1
 End of script
-Connecting to 127.0.0.1 port 1225 ... connected
-??? 220
-<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
->>> ehlo abcd
-??? 250-
-<<< 250-myhost.test.ex Hello abcd [127.0.0.1]
-??? 250-
-<<< 250-SIZE 52428800
-??? 250-
-<<< 250-PIPELINING
-??? 250
-<<< 250 HELP
->>> mail from:<userx@test.ex>\r\nrcpt to:<userx@test.ex>\r\ndata
-??? 250
-<<< 250 OK
-??? 250
-<<< 250 Accepted
-??? 354
-<<< 354 Enter message, ending with "." on a line by itself
->>> This is a test message.
->>> .\r\nrset
-??? 550
-<<< 550 Synchronization error (data after final dot)
-End of script
index dd1bdae..1efa5cd 100644 (file)
@@ -30,35 +30,3 @@ Succeeded in starting TLS
 >>> .
 +++ 1
 End of script
-Connecting to 127.0.0.1 port 1225 ... connected
-??? 220
-<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
->>> ehlo abcd
-??? 250-
-<<< 250-myhost.test.ex Hello abcd [127.0.0.1]
-??? 250-
-<<< 250-SIZE 52428800
-??? 250-
-<<< 250-PIPELINING
-??? 250-
-<<< 250-STARTTLS
-??? 250
-<<< 250 HELP
->>> starttls
-??? 220
-<<< 220 TLS go ahead
-Attempting to start TLS
-Succeeded in starting TLS
->>> mail from:<userx@test.ex>
-??? 250
-<<< 250 OK
->>> rcpt to:<userx@test.ex>
-??? 250
-<<< 250 Accepted
->>> data
-??? 354
-<<< 354 Enter message, ending with "." on a line by itself
->>> This is a test message.
->>> .\r\nrset
-+++ 1
-End of script
index dd1bdae..1efa5cd 100644 (file)
@@ -30,35 +30,3 @@ Succeeded in starting TLS
 >>> .
 +++ 1
 End of script
-Connecting to 127.0.0.1 port 1225 ... connected
-??? 220
-<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
->>> ehlo abcd
-??? 250-
-<<< 250-myhost.test.ex Hello abcd [127.0.0.1]
-??? 250-
-<<< 250-SIZE 52428800
-??? 250-
-<<< 250-PIPELINING
-??? 250-
-<<< 250-STARTTLS
-??? 250
-<<< 250 HELP
->>> starttls
-??? 220
-<<< 220 TLS go ahead
-Attempting to start TLS
-Succeeded in starting TLS
->>> mail from:<userx@test.ex>
-??? 250
-<<< 250 OK
->>> rcpt to:<userx@test.ex>
-??? 250
-<<< 250 Accepted
->>> data
-??? 354
-<<< 354 Enter message, ending with "." on a line by itself
->>> This is a test message.
->>> .\r\nrset
-+++ 1
-End of script