Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

Message::getSender creates a blank Sender header #93

Merged
weierophinney merged 3 commits into from
Feb 14, 2017
Merged

Message::getSender creates a blank Sender header #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c0b6d4
Add test for blank Sender: header.
MichaelGooden Jun 10, 2016
23af4a3
Add test for Sender header being unintentionally created.
MichaelGooden Jun 10, 2016
a6e9193
Do not unintentionally create blank Sender header.
MichaelGooden Jun 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/Message.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,10 @@ public function setSender($emailOrAddress, $name = null)
*/
public function getSender()
{
$headers = $this->getHeaders();
Copy link
Copy Markdown
Member

@Ocramius Ocramius Dec 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems unrelated: what call did cause the header to be added? You are only fixing the symptom here, IMO

Copy link
Copy Markdown
Contributor Author

@MichaelGooden MichaelGooden Dec 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling Message::getHeaderByName will create a new blank header if the requested header does not exist, as per the docbloc.

The existing strategy as per Message::getSubject is to check if the header exists before calling getHeaderByName. (The existing code also uses a local variable for $headers.)

Copy link
Copy Markdown
Contributor Author

@MichaelGooden MichaelGooden Jan 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ocramius Could you review please?

Copy link
Copy Markdown
Member

@Ocramius Ocramius Jan 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

if (!$headers->has('sender')) {
return null;
}
$header = $this->getHeaderByName('sender', __NAMESPACE__ . '\Header\Sender');
return $header->getAddress();
}
Expand Down
7 changes: 7 additions & 0 deletions test/MessageTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,13 @@ public function testSenderIsNullByDefault()
$this->assertNull($this->message->getSender());
}

public function testNullSenderDoesNotCreateHeader()
{
$sender = $this->message->getSender();
$headers = $this->message->getHeaders();
$this->assertFalse($headers->has('sender'));
}

public function testSettingSenderCreatesAddressObject()
{
$this->message->setSender('zf-devteam@example.com');
Expand Down
22 changes: 22 additions & 0 deletions test/Transport/SmtpTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,28 @@ public function testSendMinimalMail()
$this->assertContains($expectedMessage, $this->connection->getLog());
}

public function testSendMinimalMailWithoutSender()
{
$headers = new Headers();
$headers->addHeaderLine('Date', 'Sun, 10 Jun 2012 20:07:24 +0200');
$message = new Message();
$message
->setHeaders($headers)
->setFrom('ralph.schindler@zend.com', 'Ralph Schindler')
->setBody('testSendMinimalMailWithoutSender')
->addTo('zf-devteam@zend.com', 'ZF DevTeam')
;
$expectedMessage = "Date: Sun, 10 Jun 2012 20:07:24 +0200\r\n"
. "From: Ralph Schindler <ralph.schindler@zend.com>\r\n"
. "To: ZF DevTeam <zf-devteam@zend.com>\r\n"
. "\r\n"
. "testSendMinimalMailWithoutSender";

$this->transport->send($message);

$this->assertContains($expectedMessage, $this->connection->getLog());
}

public function testReceivesMailArtifacts()
{
$message = $this->getMessage();
Expand Down