AddressList does not parse address with parens in name field properly

CHANGELOG.md

@@ -6,6 +6,13 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Added
 
+- [#148](https://github.com/zendframework/zend-mail/pull/148) adds the optional constructor argument `$comment` and the method `getComment()` to the class
+  `Zend\Mail\Address`. When a comment is present, `toString()` will include it in the representation.
+
+- [#148](https://github.com/zendframework/zend-mail/pull/148) adds the method `Zend\Mail\Address::fromString(string $address, $comment = null) : Address`.
+  The method can be used to generate an instance from a string containing a `(name)?<email>` value.
+  The `$comment` argument can be used to associate a comment with the address.
+
 - [#213](https://github.com/zendframework/zend-mail/pull/213) re-adds support for PHP 5.6 and 7.0; ZF policy is never
   to bump the major version of a PHP requirement unless the package is bumping major version.
 
@@ -23,6 +30,9 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Fixed
 
+- [#148](https://github.com/zendframework/zend-mail/pull/148) fixes how `Zend\Mail\Header\AbstractAddressList` parses address values, ensuring
+  that they now retain any address comment discovered to include in the generated `Zend\Mail\Address` instances.
+
 - [#147](https://github.com/zendframework/zend-mail/pull/147) fixes how address lists are parsed, expanding the functionality to allow either
   `,` or `;` delimiters (or both in combination).
 

src/Address.php

@@ -12,17 +12,56 @@
 
 class Address implements Address\AddressInterface
 {
+    protected $comment;
     protected $email;
     protected $name;
 
+    /**
+     * Create an instance from a string value.
+     *
+     * Parses a string representing a single address. If it is a valid format,
+     * it then creates and returns an instance of itself using the name and
+     * email it has parsed from the value.
+     *
+     * @param string $address
+     * @param null|string $comment Comment associated with the address, if any.
+     * @throws Exception\InvalidArgumentException
+     * @return self
+     */
+    public static function fromString($address, $comment = null)
+    {
+        if (! preg_match('/^((?P<name>.*)<(?P<namedEmail>[^>]+)>|(?P<email>.+))$/', $address, $matches)) {
+            throw new Exception\InvalidArgumentException('Invalid address format');
+        }
+
+        $name = null;
+        if (isset($matches['name'])) {
+            $name = trim($matches['name']);
+        }
+        if (empty($name)) {
+            $name = null;
+        }
+
+        if (isset($matches['namedEmail'])) {
+            $email = $matches['namedEmail'];
+        }
+        if (isset($matches['email'])) {
+            $email = $matches['email'];
+        }
+        $email = trim($email);
+
+        return new static($email, $name, $comment);
+    }
+
     /**
      * Constructor
      *
      * @param  string $email
      * @param  null|string $name
+     * @param  null|string $comment
      * @throws Exception\InvalidArgumentException
      */
-    public function __construct($email, $name = null)
+    public function __construct($email, $name = null, $comment = null)
     {
         $emailAddressValidator = new EmailAddressValidator(Hostname::ALLOW_DNS | Hostname::ALLOW_LOCAL);
         if (! is_string($email) || empty($email)) {
@@ -51,6 +90,10 @@ public function __construct($email, $name = null)
         }
 
         $this->email = $email;
+
+        if (null !== $comment) {
+            $this->comment = $comment;
+        }
     }
 
     /**
@@ -73,19 +116,50 @@ public function getName()
         return $this->name;
     }
 
+    /**
+     * Retrieve comment, if any
+     *
+     * @return null|string
+     */
+    public function getComment()
+    {
+        return $this->comment;
+    }
+
     /**
      * String representation of address
      *
      * @return string
      */
     public function toString()
     {
-        $string = '<' . $this->getEmail() . '>';
-        $name   = $this->getName();
+        $string = sprintf('<%s>', $this->getEmail());
+        $name   = $this->constructName();
         if (null === $name) {
             return $string;
         }
 
-        return $name . ' ' . $string;
+        return sprintf('%s %s', $name, $string);
+    }
+
+    /**
+     * Constructs the name string
+     *
+     * If a comment is present, appends the comment (commented using parens) to
+     * the name before returning it; otherwise, returns just the name.
+     *
+     * @return null|string
+     */
+    private function constructName()
+    {
+        $name = $this->getName();
+        $comment = $this->getComment();
+
+        if ($comment === null || $comment === '') {
+            return $name;
+        }
+
+        $string = sprintf('%s (%s)', $name, $comment);
+        return trim($string);
     }
 }

src/AddressList.php

@@ -88,32 +88,13 @@ public function addMany(array $addresses)
      *  - dev@zf.com
      *
      * @param string $address
+     * @param null|string $comment Comment associated with the address, if any.
      * @throws Exception\InvalidArgumentException
      * @return AddressList
      */
-    public function addFromString($address)
+    public function addFromString($address, $comment = null)
     {
-        if (! preg_match('/^((?P<name>.*)<(?P<namedEmail>[^>]+)>|(?P<email>.+))$/', $address, $matches)) {
-            throw new Exception\InvalidArgumentException('Invalid address format');
-        }
-
-        $name = null;
-        if (isset($matches['name'])) {
-            $name = trim($matches['name']);
-        }
-        if (empty($name)) {
-            $name = null;
-        }
-
-        if (isset($matches['namedEmail'])) {
-            $email = $matches['namedEmail'];
-        }
-        if (isset($matches['email'])) {
-            $email = $matches['email'];
-        }
-        $email = trim($email);
-
-        return $this->add($email, $name);
+        $this->add(Address::fromString($address, $comment));
     }
 
     /**

src/Header/AbstractAddressList.php

@@ -7,6 +7,7 @@
 
 namespace Zend\Mail\Header;
 
+use Zend\Mail\Address;
 use Zend\Mail\AddressList;
 use Zend\Mail\Headers;
 
@@ -53,38 +54,45 @@ public static function fromString($headerLine)
         $values = AddressListParser::parse($fieldValue);
 
         $wasEncoded = false;
-        array_walk(
-            $values,
-            function (&$value) use (&$wasEncoded) {
+        $addresses = array_map(
+            function ($value) use (&$wasEncoded) {
                 $decodedValue = HeaderWrap::mimeDecodeValue($value);
                 $wasEncoded = $wasEncoded || ($decodedValue !== $value);
+
                 $value = trim($decodedValue);
+
+                $comments = self::getComments($value);
                 $value = self::stripComments($value);
+
                 $value = preg_replace(
                     [
-                        '#(?<!\\\)"(.*)(?<!\\\)"#', //quoted-text
-                        '#\\\([\x01-\x09\x0b\x0c\x0e-\x7f])#' //quoted-pair
+                        '#(?<!\\\)"(.*)(?<!\\\)"#',            // quoted-text
+                        '#\\\([\x01-\x09\x0b\x0c\x0e-\x7f])#', // quoted-pair
                     ],
                     [
                         '\\1',
-                        '\\1'
+                        '\\1',
                     ],
                     $value
                 );
-            }
+
+                return empty($value) ? null : Address::fromString($value, $comments);
+            },
+            $values
         );
+        $addresses = array_filter($addresses);
+
         $header = new static();
         if ($wasEncoded) {
             $header->setEncoding('UTF-8');
         }
 
-        $values = array_filter($values);
-
         /** @var AddressList $addressList */
         $addressList = $header->getAddressList();
-        foreach ($values as $address) {
-            $addressList->addFromString($address);
+        foreach ($addresses as $address) {
+            $addressList->add($address);
         }
+
         return $header;
     }
 
@@ -194,7 +202,38 @@ public function toString()
         return (empty($value)) ? '' : sprintf('%s: %s', $name, $value);
     }
 
-    // Supposed to be private, protected as a workaround for PHP bug 68194
+    /**
+     * Retrieve comments from value, if any.
+     *
+     * Supposed to be private, protected as a workaround for PHP bug 68194
+     *
+     * @param string $value
+     * @return string
+     */
+    protected static function getComments($value)
+    {
+        $matches = [];
+        preg_match_all(
+            '/\\(
+                (?P<comment>(
+                    \\\\.|
+                    [^\\\\)]
+                )+)
+            \\)/x',
+            $value,
+            $matches
+        );
+        return isset($matches['comment']) ? implode(', ', $matches['comment']) : '';
+    }
+
+    /**
+     * Strip all comments from value, if any.
+     *
+     * Supposed to be private, protected as a workaround for PHP bug 68194
+     *
+     * @param string $value
+     * @return void
+     */
     protected static function stripComments($value)
     {
         return preg_replace(

test/AddressListTest.php

@@ -19,7 +19,7 @@
  */
 class AddressListTest extends TestCase
 {
-    /** @var AddressList $list */
+    /** @var AddressList */
     private $list;
 
     public function setUp()
@@ -95,6 +95,18 @@ public function testCanAddManyAddressesAtOnce()
         $this->assertTrue($this->list->has('fw-announce@lists.zend.com'));
     }
 
+    public function testLosesParensInName()
+    {
+        $header = '"Supports (E-mail)" <support@example.org>';
+
+        $to = Header\To::fromString('To:' . $header);
+        $addressList = $to->getAddressList();
+        $address = $addressList->get('support@example.org');
+        $this->assertEquals('Supports', $address->getName());
+        $this->assertEquals('E-mail', $address->getComment());
+        $this->assertEquals('support@example.org', $address->getEmail());
+    }
+
     public function testDoesNotStoreDuplicatesAndFirstWins()
     {
         $addresses = [