Don't send QUIT while DATA is in progress

If the connection is closed in the middle of a DATA command, for example
from a destructor after SIGTERM, the server takes QUIT as part of the
message and the connection only ends when it times out. SMTP now keeps
the last command it sent in a state property, and quit() just closes the
connection when that command is DATA, as discussed in #3121.

Fixes #2994

src/SMTP.php

@@ -275,6 +275,14 @@ class SMTP
      */
     protected $last_reply = '';
 
+    /**
+     * The last command sent to the server.
+     * Used to avoid sending QUIT while a DATA command is still in progress.
+     *
+     * @var string
+     */
+    protected $state = '';
+
     /**
      * Output debugging info via a user-selected method.
      *
@@ -763,6 +771,7 @@ class SMTP
     {
         $this->server_caps = null;
         $this->helo_rply = null;
+        $this->state = '';
         if (is_resource($this->smtp_conn)) {
             //Close the connection and cleanup
             fclose($this->smtp_conn);
@@ -1021,7 +1030,12 @@ class SMTP
      */
     public function quit($close_on_error = true)
     {
-        $noerror = $this->sendCommand('QUIT', 'QUIT', 221);
+        if ($this->state === 'DATA') {
+            //The server would take QUIT as part of an unfinished message, so just close the connection
+            $noerror = true;
+        } else {
+            $noerror = $this->sendCommand('QUIT', 'QUIT', 221);
+        }
         $err = $this->error; //Save any error
         if ($noerror || $close_on_error) {
             $this->close();
@@ -1124,6 +1138,7 @@ class SMTP
 
             return false;
         }
+        $this->state = $command;
         $this->client_send($commandstring . static::LE, $command);
 
         $this->last_reply = $this->get_lines();

test/PHPMailer/PHPMailerTest.php

@@ -1116,6 +1116,37 @@ EOT;
         self::assertFalse($smtp->mail("somewhere\nbad"), 'Bad SMTP command containing breaks accepted');
     }
 
+    /**
+     * Closing a connection in the middle of DATA must not send QUIT, as the server would take it as message content.
+     */
+    public function testQuitDuringData()
+    {
+        self::assertTrue($this->Mail->smtpConnect(), 'SMTP connect failed');
+        $smtp = $this->Mail->getSMTPInstance();
+        //Don't wait long for a reply that should never be requested
+        $smtp->Timelimit = 5;
+        self::assertTrue($smtp->mail('phpunit@example.com'));
+        self::assertTrue($smtp->recipient('somebody@example.com'));
+
+        //Start DATA without sending the message, as if the send had been interrupted
+        $reflMethod = new \ReflectionMethod($smtp, 'sendCommand');
+        (\PHP_VERSION_ID < 80100) && $reflMethod->setAccessible(true);
+        self::assertTrue($reflMethod->invoke($smtp, 'DATA', 'DATA', 354));
+        (\PHP_VERSION_ID < 80100) && $reflMethod->setAccessible(false);
+
+        $sent = '';
+        $smtp->setDebugLevel(SMTP::DEBUG_CLIENT);
+        $smtp->setDebugOutput(
+            static function ($str) use (&$sent) {
+                $sent .= $str;
+            }
+        );
+        $smtp->quit();
+
+        self::assertStringNotContainsString('QUIT', $sent, 'QUIT was sent during DATA');
+        self::assertFalse($smtp->connected(), 'Connection was not closed');
+    }
+
     /**
      * Tests setting and retrieving ConfirmReadingTo address, also known as "read receipt" address.
      */