Merge branch 'feature/172-connection-timeout'

Close #172
zendframework · Jun 6, 2018 · 2ff0031 · 2ff0031
2 parents 374a53f + 709e342
commit 2ff0031
Show file tree

Hide file tree

Showing 5 changed files with 182 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Added
 
+- [#172](https://github.com/zendframework/zend-mail/pull/172) adds the flag `connection_time_limit` to the possible `Zend\Mail\Transport\Smtp` options.
+  This flag, when provided as a positive integer, and in conjunction with the `use_complete_quit` flag, will
+  reconnect to the server after the specified interval.
+
 - [#166](https://github.com/zendframework/zend-mail/pull/166) adds functionality for handling `References` and `In-Reply-To` headers.
 
 - [#148](https://github.com/zendframework/zend-mail/pull/148) adds the optional constructor argument `$comment` and the method `getComment()` to the class

diff --git a/docs/book/transport/smtp-authentication.md b/docs/book/transport/smtp-authentication.md
@@ -132,7 +132,7 @@ By default, every `Zend\Mail\Protocol\Smtp\*` class tries to disconnect from
 the STMP server by sending a `QUIT` command and expecting a `221` (_Service
 closing transmission channel_) response code.  This is done automatically at
 object destruction (via the `__destruct()` method), and can generate errors
-with SMTP servers like [Posftix](http://www.postfix.org/postconf.5.html#smtp_connection_reuse_time_limit)
+with SMTP servers like [Postfix](http://www.postfix.org/postconf.5.html#smtp_connection_reuse_time_limit)
 that implement a reuse time limit:
 
 ```php
@@ -150,8 +150,7 @@ exit;
 // Fatal error: Uncaught Zend\Mail\Protocol\Exception\RuntimeException: Could not read from 127.0.0.1 in ./zend-mail/src/Protocol/AbstractProtocol.php:301
 ```
 
-To avoid this error, you can configure any `Zend\Mail\Protocol\Smtp\*` instance
-to close the connection with the SMTP server without the `QUIT` command:
+To avoid this error, you can set a time limit for the SMTP connection in `SmtpOptions`: 
 
 ```php
 use Zend\Mail\Transport\Smtp as SmtpTransport;
@@ -160,29 +159,31 @@ use Zend\Mail\Transport\SmtpOptions;
 // Setup SMTP transport to exit without the `QUIT` command
 $transport = new SmtpTransport();
 $options   = new SmtpOptions([
-    'name'              => 'localhost.localdomain',
-    'host'              => '127.0.0.1',
-    'connection_class'  => 'plain',
-    'connection_config' => [
-        'username'          => 'user',
-        'password'          => 'pass',
-        'use_complete_quit' => false,
+    'name'                  => 'localhost.localdomain',
+    'host'                  => '127.0.0.1',
+    'connection_time_limit' => 300, // recreate the connection 5 minutes after connect()
+    'connection_class'      => 'plain',
+    'connection_config'     => [
+        'username'            => 'user',
+        'password'            => 'pass',
+        'use_complete_quit'   => false, // Dont send 'QUIT' on __destruct()
     ],
 ]);
 $transport->setOptions($options);
 ```
 
+Setting `connection_time_limit` will automatically set `use_complete_quit` to `false`, 
+so the connection with the SMTP server will be closed without the `QUIT` command.
+
 > ### NOTE: recreate old connection
 >
-> The flag described above aims to avoid errors that you cannot manage from PHP.
+> The `use_complete_quit` flag described above aims to avoid errors that you
+> cannot manage from PHP.
 >
 > If you deal with SMTP servers that exhibit this behavior from within
-> long-running scripts, you will need to:
->
-> 1. Trace the time elapsed since the creation of the connection.
-> 1. Close and reopen the connection if the elapsed time exceeds the reuse
->    time limit
+> long-running scripts, you SHOULD use the flag along with the
+> `connection_time_limit` flag to ensure you recreate the connection.
+
+> ### Since 2.10.0
 >
-> As an example, you can perform these steps by applying a proxy class that
-> wraps the real `Zend\Mail\Protocol\Smtp\*` instance to track the construction
-> time, and then close and open the connection when needed.
+> The `connection_time_limit` flag has been available since 2.10.0.
diff --git a/src/Transport/Smtp.php b/src/Transport/Smtp.php
@@ -46,6 +46,13 @@ class Smtp implements TransportInterface
      */
     protected $plugins;
 
+    /**
+     * When did we connect to the server?
+     *
+     * @var int|null
+     */
+    protected $connectedTime;
+
     /**
      * Constructor.
      *
@@ -166,15 +173,18 @@ public function plugin($name, array $options = null)
      */
     public function __destruct()
     {
-        if ($this->connection instanceof Protocol\Smtp) {
-            try {
-                $this->connection->quit();
-            } catch (ProtocolException\ExceptionInterface $e) {
-                // ignore
-            }
-            if ($this->autoDisconnect) {
-                $this->connection->disconnect();
-            }
+        if (! $this->getConnection() instanceof Protocol\Smtp) {
+            return;
+        }
+
+        try {
+            $this->getConnection()->quit();
+        } catch (ProtocolException\ExceptionInterface $e) {
+            // ignore
+        }
+
+        if ($this->autoDisconnect) {
+            $this->getConnection()->disconnect();
         }
     }
 
@@ -186,6 +196,11 @@ public function __destruct()
     public function setConnection(Protocol\AbstractProtocol $connection)
     {
         $this->connection = $connection;
+        if (($connection instanceof Protocol\Smtp)
+            && ($this->getOptions()->getConnectionTimeLimit() !== null)
+        ) {
+            $connection->setUseCompleteQuit(false);
+        }
     }
 
     /**
@@ -195,6 +210,13 @@ public function setConnection(Protocol\AbstractProtocol $connection)
      */
     public function getConnection()
     {
+        $timeLimit = $this->getOptions()->getConnectionTimeLimit();
+        if ($timeLimit !== null
+            && $this->connectedTime !== null
+            && ((time() - $this->connectedTime) > $timeLimit)
+        ) {
+            $this->connection = null;
+        }
         return $this->connection;
     }
 
@@ -205,8 +227,9 @@ public function getConnection()
      */
     public function disconnect()
     {
-        if (! empty($this->connection) && ($this->connection instanceof Protocol\Smtp)) {
-            $this->connection->disconnect();
+        if ($this->getConnection() instanceof Protocol\Smtp) {
+            $this->getConnection()->disconnect();
+            $this->connectedTime = null;
         }
     }
 
@@ -354,8 +377,8 @@ protected function lazyLoadConnection()
         $config           = $options->getConnectionConfig();
         $config['host']   = $options->getHost();
         $config['port']   = $options->getPort();
-        $connection       = $this->plugin($options->getConnectionClass(), $config);
-        $this->connection = $connection;
+
+        $this->setConnection($this->plugin($options->getConnectionClass(), $config));
 
         return $this->connect();
     }
@@ -372,6 +395,9 @@ protected function connect()
         }
 
         $this->connection->connect();
+
+        $this->connectedTime = time();
+
         $this->connection->helo($this->getOptions()->getName());
 
         return $this->connection;

diff --git a/src/Transport/SmtpOptions.php b/src/Transport/SmtpOptions.php
@@ -39,6 +39,14 @@ class SmtpOptions extends AbstractOptions
      */
     protected $port = 25;
 
+    /**
+     * The timeout in seconds for the SMTP connection
+     * (Use null to disable it)
+     *
+     * @var int|null
+     */
+    protected $connectionTimeLimit;
+
     /**
      * Return the local client hostname
      *
@@ -176,4 +184,25 @@ public function setPort($port)
         $this->port = $port;
         return $this;
     }
+
+    /**
+     * @return int|null
+     */
+    public function getConnectionTimeLimit()
+    {
+        return $this->connectionTimeLimit;
+    }
+
+    /**
+     * @param int|null $seconds
+     * @return self
+     */
+    public function setConnectionTimeLimit($seconds)
+    {
+        $this->connectionTimeLimit = $seconds === null
+            ? null
+            : (int) $seconds;
+
+        return $this;
+    }
 }
diff --git a/test/Transport/SmtpTest.php b/test/Transport/SmtpTest.php
@@ -10,6 +10,8 @@
 use PHPUnit\Framework\TestCase;
 use Zend\Mail\Headers;
 use Zend\Mail\Message;
+use Zend\Mail\Protocol\Smtp as SmtpProtocol;
+use Zend\Mail\Protocol\SmtpPluginManager;
 use Zend\Mail\Transport\Envelope;
 use Zend\Mail\Transport\Smtp;
 use Zend\Mail\Transport\SmtpOptions;
@@ -255,4 +257,92 @@ public function testDisconnectSendReconnects()
         $this->transport->send($this->getMessage());
         $this->assertTrue($this->connection->hasSession());
     }
+
+    public function testAutoReconnect()
+    {
+        $options = new SmtpOptions();
+        $options->setConnectionTimeLimit(5 * 3600);
+
+        $this->transport->setOptions($options);
+
+        // Mock the connection
+        $connectionMock = $this->getMockBuilder(SmtpProtocol::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['connect', 'helo', 'hasSession', 'mail', 'rcpt', 'data', 'rset'])
+            ->getMock();
+
+        $connectionMock
+            ->expects(self::exactly(2))
+            ->method('hasSession')
+            ->willReturnOnConsecutiveCalls(
+                false,
+                true
+            );
+
+        $connectionMock
+            ->expects(self::exactly(2))
+            ->method('connect');
+
+        $connectionMock
+            ->expects(self::exactly(2))
+            ->method('helo');
+
+        $connectionMock
+            ->expects(self::exactly(3))
+            ->method('mail');
+
+        $connectionMock
+            ->expects(self::exactly(9))
+            ->method('rcpt');
+
+        $connectionMock
+            ->expects(self::exactly(3))
+            ->method('data');
+
+        $connectionMock
+            ->expects(self::exactly(1))
+            ->method('rset');
+
+        $this->transport->setConnection($connectionMock);
+
+        // Mock the plugin manager so that lazyLoadConnection() works
+        $pluginManagerMock = $this->getMockBuilder(SmtpPluginManager::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['get'])
+            ->getMock();
+
+        $pluginManagerMock
+            ->expects(self::once())
+            ->method('get')
+            ->willReturn($connectionMock);
+
+        $this->transport->setPluginManager($pluginManagerMock);
+
+
+        // Send the first email - first connect()
+        $this->transport->send($this->getMessage());
+
+        // Check that the connectedTime was set properly
+        $reflClass             = new \ReflectionClass($this->transport);
+        $connectedTimeProperty = $reflClass->getProperty('connectedTime');
+
+        self::assertNotNull($connectedTimeProperty);
+        $connectedTimeProperty->setAccessible(true);
+        $connectedTimeAfterFirstMail = $connectedTimeProperty->getValue($this->transport);
+        $this->assertNotNull($connectedTimeAfterFirstMail);
+
+
+        // Send the second email - no new connect()
+        $this->transport->send($this->getMessage());
+
+        // Make sure that there was no new connect() (and no new timestamp was written)
+        $this->assertEquals($connectedTimeAfterFirstMail, $connectedTimeProperty->getValue($this->transport));
+
+
+        // Manipulate the timestamp to trigger the auto-reconnect
+        $connectedTimeProperty->setValue($this->transport, time() - 10 * 3600);
+
+        // Send the third email - it should trigger a new connect()
+        $this->transport->send($this->getMessage());
+    }
 }