feature #799 Dependency-inject local-domain into AbstractSmtpTranspor…

…t (c960657)

This PR was merged into the 6.0-dev branch.

Discussion
----------

Dependency-inject local-domain into AbstractSmtpTransport

Instead of accessing `$_SERVER['SERVER_NAME']` directly from `Swift_Transport_AbstractSmtpTransport`, it would be cleaner to inject `localDomain` through the dependency injection container.

Commits
-------

9658bab Dependency-inject local-domain into AbstractSmtpTransport

lib/classes/Swift/Transport/AbstractSmtpTransport.php

@@ -38,12 +38,13 @@ abstract protected function getBufferParams();
      *
      * @param Swift_Transport_IoBuffer     $buf
      * @param Swift_Events_EventDispatcher $dispatcher
+     * @param string                       $localDomain
      */
-    public function __construct(Swift_Transport_IoBuffer $buf, Swift_Events_EventDispatcher $dispatcher)
+    public function __construct(Swift_Transport_IoBuffer $buf, Swift_Events_EventDispatcher $dispatcher, $localDomain)
     {
         $this->eventDispatcher = $dispatcher;
         $this->buffer = $buf;
-        $this->lookupHostname();
+        $this->setLocalDomain($localDomain);
     }
 
     /**
@@ -52,15 +53,24 @@ public function __construct(Swift_Transport_IoBuffer $buf, Swift_Events_EventDis
      * This should be a fully-qualified domain name and should be truly the domain
      * you're using.
      *
-     * If your server doesn't have a domain name, use the IP in square
-     * brackets (i.e. [127.0.0.1]).
+     * If your server does not have a domain name, use the IP address. This will
+     * automatically be wrapped in square brackets as described in RFC 5321,
+     * section 4.1.3.
      *
      * @param string $domain
      *
      * @return $this
      */
     public function setLocalDomain($domain)
     {
+        if (substr($domain, 0, 1) !== '[') {
+            if (filter_var($domain, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
+                $domain = '['.$domain.']';
+            } elseif (filter_var($domain, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
+                $domain = '[IPv6:'.$domain.']';
+            }
+        }
+
         $this->domain = $domain;
 
         return $this;
@@ -69,6 +79,9 @@ public function setLocalDomain($domain)
     /**
      * Get the name of the domain Swift will identify as.
      *
+     * If an IP address was specified, this will be returned wrapped in square
+     * brackets as described in RFC 5321, section 4.1.3.
+     *
      * @return string
      */
     public function getLocalDomain()
@@ -481,34 +494,6 @@ private function sendBcc(Swift_Mime_SimpleMessage $message, $reversePath, array
         return $sent;
     }
 
-    /** Try to determine the hostname of the server this is run on */
-    private function lookupHostname()
-    {
-        if (!empty($_SERVER['SERVER_NAME']) && $this->isFqdn($_SERVER['SERVER_NAME'])) {
-            $this->domain = $_SERVER['SERVER_NAME'];
-        } elseif (!empty($_SERVER['SERVER_ADDR'])) {
-            // Set the address literal tag (See RFC 5321, section: 4.1.3)
-            if (false === strpos($_SERVER['SERVER_ADDR'], ':')) {
-                $prefix = ''; // IPv4 addresses are not tagged.
-            } else {
-                $prefix = 'IPv6:'; // Adding prefix in case of IPv6.
-            }
-
-            $this->domain = sprintf('[%s%s]', $prefix, $_SERVER['SERVER_ADDR']);
-        }
-    }
-
-    /** Determine is the $hostname is a fully-qualified name */
-    private function isFqdn($hostname)
-    {
-        // We could do a really thorough check, but there's really no point
-        if (false !== $dotPos = strpos($hostname, '.')) {
-            return ($dotPos > 0) && ($dotPos != strlen($hostname) - 1);
-        }
-
-        return false;
-    }
-
     /**
      * Destructor.
      */

lib/classes/Swift/Transport/EsmtpTransport.php

@@ -51,10 +51,11 @@ class Swift_Transport_EsmtpTransport extends Swift_Transport_AbstractSmtpTranspo
      * @param Swift_Transport_IoBuffer       $buf
      * @param Swift_Transport_EsmtpHandler[] $extensionHandlers
      * @param Swift_Events_EventDispatcher   $dispatcher
+     * @param string                         $localDomain
      */
-    public function __construct(Swift_Transport_IoBuffer $buf, array $extensionHandlers, Swift_Events_EventDispatcher $dispatcher)
+    public function __construct(Swift_Transport_IoBuffer $buf, array $extensionHandlers, Swift_Events_EventDispatcher $dispatcher, $localDomain)
     {
-        parent::__construct($buf, $dispatcher);
+        parent::__construct($buf, $dispatcher, $localDomain);
         $this->setExtensionHandlers($extensionHandlers);
     }
 

lib/classes/Swift/Transport/SendmailTransport.php

@@ -36,10 +36,11 @@ class Swift_Transport_SendmailTransport extends Swift_Transport_AbstractSmtpTran
      *
      * @param Swift_Transport_IoBuffer     $buf
      * @param Swift_Events_EventDispatcher $dispatcher
+     * @param string                       $localDomain
      */
-    public function __construct(Swift_Transport_IoBuffer $buf, Swift_Events_EventDispatcher $dispatcher)
+    public function __construct(Swift_Transport_IoBuffer $buf, Swift_Events_EventDispatcher $dispatcher, $localDomain)
     {
-        parent::__construct($buf, $dispatcher);
+        parent::__construct($buf, $dispatcher, $localDomain);
     }
 
     /**

lib/dependency_maps/transport_deps.php

@@ -1,19 +1,27 @@
 <?php
 
 Swift_DependencyContainer::getInstance()
+    ->register('transport.localdomain')
+    // As SERVER_NAME can come from the user in certain configurations, check that
+    // it does not contain forbidden characters (see RFC 952 and RFC 2181). Use
+    // preg_replace() instead of preg_match() to prevent DoS attacks with long host names.
+    ->asValue(!empty($_SERVER['SERVER_NAME']) && preg_replace('/(?:^\[)?[a-zA-Z0-9-:\]_]+\.?/', '', $_SERVER['SERVER_NAME']) === '' ? trim($_SERVER['SERVER_NAME'], '[]') : '127.0.0.1')
+
     ->register('transport.smtp')
     ->asNewInstanceOf('Swift_Transport_EsmtpTransport')
     ->withDependencies(array(
         'transport.buffer',
         array('transport.authhandler'),
         'transport.eventdispatcher',
+        'transport.localdomain',
     ))
 
     ->register('transport.sendmail')
     ->asNewInstanceOf('Swift_Transport_SendmailTransport')
     ->withDependencies(array(
         'transport.buffer',
         'transport.eventdispatcher',
+        'transport.localdomain',
     ))
 
     ->register('transport.loadbalanced')

tests/unit/Swift/Transport/AbstractSmtpTest.php

@@ -103,7 +103,7 @@ public function testStartSendsHeloToInitiate()
             ->andReturn("220 some.server.tld bleh\r\n");
         $buf->shouldReceive('write')
             ->once()
-            ->with('~^HELO .*?\r\n$~D')
+            ->with('~^HELO example.org\r\n$~D')
             ->andReturn(1);
         $buf->shouldReceive('readLine')
             ->once()
@@ -131,7 +131,7 @@ public function testInvalidHeloResponseCausesException()
             ->andReturn("220 some.server.tld bleh\r\n");
         $buf->shouldReceive('write')
             ->once()
-            ->with('~^HELO .*?\r\n$~D')
+            ->with('~^HELO example.org\r\n$~D')
             ->andReturn(1);
         $buf->shouldReceive('readLine')
             ->once()
@@ -1220,6 +1220,27 @@ public function testPingOnDeadConnection()
         $this->assertFalse($smtp->isStarted());
     }
 
+    public function testSetLocalDomain()
+    {
+        $buf = $this->getBuffer();
+        $smtp = $this->getTransport($buf);
+
+        $smtp->setLocalDomain('example.com');
+        $this->assertEquals('example.com', $smtp->getLocalDomain());
+
+        $smtp->setLocalDomain('192.168.0.1');
+        $this->assertEquals('[192.168.0.1]', $smtp->getLocalDomain());
+
+        $smtp->setLocalDomain('[192.168.0.1]');
+        $this->assertEquals('[192.168.0.1]', $smtp->getLocalDomain());
+
+        $smtp->setLocalDomain('fd00::');
+        $this->assertEquals('[IPv6:fd00::]', $smtp->getLocalDomain());
+
+        $smtp->setLocalDomain('[IPv6:fd00::]');
+        $this->assertEquals('[IPv6:fd00::]', $smtp->getLocalDomain());
+    }
+
     protected function getBuffer()
     {
         return $this->getMockery('Swift_Transport_IoBuffer')->shouldIgnoreMissing();

tests/unit/Swift/Transport/EsmtpTransport/ExtensionSupportTest.php

@@ -141,7 +141,7 @@ public function testExtensionsCanModifyMailFromParams()
     {
         $buf = $this->getBuffer();
         $dispatcher = $this->createEventDispatcher();
-        $smtp = new Swift_Transport_EsmtpTransport($buf, array(), $dispatcher);
+        $smtp = new Swift_Transport_EsmtpTransport($buf, array(), $dispatcher, 'example.org');
         $ext1 = $this->getMockery('Swift_Transport_EsmtpHandler')->shouldIgnoreMissing();
         $ext2 = $this->getMockery('Swift_Transport_EsmtpHandler')->shouldIgnoreMissing();
         $ext3 = $this->getMockery('Swift_Transport_EsmtpHandler')->shouldIgnoreMissing();
@@ -243,7 +243,7 @@ public function testExtensionsCanModifyRcptParams()
     {
         $buf = $this->getBuffer();
         $dispatcher = $this->createEventDispatcher();
-        $smtp = new Swift_Transport_EsmtpTransport($buf, array(), $dispatcher);
+        $smtp = new Swift_Transport_EsmtpTransport($buf, array(), $dispatcher, 'example.org');
         $ext1 = $this->getMockery('Swift_Transport_EsmtpHandler')->shouldIgnoreMissing();
         $ext2 = $this->getMockery('Swift_Transport_EsmtpHandler')->shouldIgnoreMissing();
         $ext3 = $this->getMockery('Swift_Transport_EsmtpHandler')->shouldIgnoreMissing();

tests/unit/Swift/Transport/EsmtpTransportTest.php

@@ -8,7 +8,7 @@ protected function getTransport($buf, $dispatcher = null)
             $dispatcher = $this->createEventDispatcher();
         }
 
-        return new Swift_Transport_EsmtpTransport($buf, array(), $dispatcher);
+        return new Swift_Transport_EsmtpTransport($buf, array(), $dispatcher, 'example.org');
     }
 
     public function testHostCanBeSetAndFetched()

tests/unit/Swift/Transport/SendmailTransportTest.php

@@ -7,7 +7,7 @@ protected function getTransport($buf, $dispatcher = null, $command = '/usr/sbin/
         if (!$dispatcher) {
             $dispatcher = $this->createEventDispatcher();
         }
-        $transport = new Swift_Transport_SendmailTransport($buf, $dispatcher);
+        $transport = new Swift_Transport_SendmailTransport($buf, $dispatcher, 'example.org');
         $transport->setCommand($command);
 
         return $transport;
@@ -18,7 +18,7 @@ protected function getSendmail($buf, $dispatcher = null)
         if (!$dispatcher) {
             $dispatcher = $this->createEventDispatcher();
         }
-        $sendmail = new Swift_Transport_SendmailTransport($buf, $dispatcher);
+        $sendmail = new Swift_Transport_SendmailTransport($buf, $dispatcher, 'example.org');
 
         return $sendmail;
     }