AddressList does not parse address with parens in name field properly #148
Conversation
|
broken by #12, but it was likely even more broken before that PR. |
|
i think consider such code: $name = "Supports (E-mail)";
$email = 'support@example.org';
$address = new \Zend\Mail\Address($email, $name);
// prints: Supports (E-mail) <support@example.org>
echo $address->toString(), "\n";
$to = Header\To::fromString('To:' . $address->toString());
// prints: To: Supports <support@example.org>
echo $to->toString(), "\n"; |
weierophinney
force-pushed
the
address-parser-loses-comments
branch
from
9d5d8bb to
4d7550f
Compare
|
In looking into this, I found this line: We're currently explicitly stripping comments from names. As such, I think you're correct: we should aggregate the comment separately, and then have |
This patch accomplishes several things: - `Address` adds a method, `getComment()`, mapping to a new, optional constructor value `$comment`. When present, the generated address will include the comment: `Name (Comment) <Email>`. - It moves the logic for parsing an address value in order to create an `Address` to the `Address` class itself. - `addFromString()` (and the new `Address::fromString()`) now accepts an optional `$comment = null` value; when present, the value is passed to the `Address` constructor during instantiation. - `AbstractAddressList` adds a new method, `getComments()`. `fromString()` was updated to replace its `array_walk()` with an `array_map()` that calls this method, and uses the value to pass to `Address::fromString()` (along with the filtered `$value`). This ensures we can aggregate the comments, if any, with the address they belong to.
|
|
||
| $name = null; | ||
| if (isset($matches['name'])) { | ||
| $name = trim($matches['name']); |
There was a problem hiding this comment.
you could write as: $name = trim($matches['name']) ?: null and drop next if block ;)
There was a problem hiding this comment.
We'll get an error if $matches['name'] is not set, however. (Tried it previously which is why I went with this.)
There was a problem hiding this comment.
nah, $name is assigned null on line 37 before the isset block.
| $address = $addressList->get('support@example.org'); | ||
| $this->assertEquals('Supports', $address->getName()); | ||
| $this->assertEquals('E-mail', $address->getComment()); | ||
| $this->assertEquals('support@example.org', $address->getEmail()); |
There was a problem hiding this comment.
not testing the toString?
There was a problem hiding this comment.
Probably should! I was primarily ensuring that the various accessors returned what was expected.
this gets parsed as
Support, not as expectedSupports (E-mail)