diff --git a/src/Object/Email.php b/src/Object/Email.php index 9f78763127..008e68c4bc 100644 --- a/src/Object/Email.php +++ b/src/Object/Email.php @@ -137,10 +137,10 @@ class Email implements IEmail foreach ($this->additionalMailHeader as $name => $values) { if (is_array($values)) { foreach ($values as $value) { - $headerString .= $name . ': ' . $value . '\n'; + $headerString .= "$name : $value\n"; } } else { - $headerString .= $name . ': ' . $values . '\n'; + $headerString .= "$name : $values\n"; } } return $headerString; diff --git a/src/Util/Emailer.php b/src/Util/Emailer.php index 0594937ec8..ed6c7b331b 100644 --- a/src/Util/Emailer.php +++ b/src/Util/Emailer.php @@ -134,6 +134,17 @@ class Emailer return true; } + // @see https://github.com/friendica/friendica/issues/9142 + $countMessageId = 0; + foreach ($email->getAdditionalMailHeader() as $name => $value) { + if (strtolower($name) == 'message-id') { + $countMessageId += count($value); + } + } + if ($countMessageId > 0) { + $this->logger->warning('More than one Message-ID found - RFC violation', ['email' => $email]); + } + $email_textonly = false; if (!empty($email->getRecipientUid())) { $email_textonly = $this->pConfig->get($email->getRecipientUid(), 'system', 'email_textonly'); @@ -197,15 +208,34 @@ class Emailer return true; } - $res = mail( + $res = $this->mail( $hookdata['to'], $hookdata['subject'], $hookdata['body'], $hookdata['headers'], $hookdata['parameters'] ); + $this->logger->debug('header ' . 'To: ' . $email->getToAddress() . '\n' . $messageHeader); $this->logger->debug('return value ' . (($res) ? 'true' : 'false')); + return $res; } + + /** + * Wrapper around the mail() method (mainly used to overwrite for tests) + * @see mail() + * + * @param string $to Recipient of this mail + * @param string $subject Subject of this mail + * @param string $body Message body of this mail + * @param string $headers Headers of this mail + * @param string $parameters Additional (sendmail) parameters of this mail + * + * @return boolean true if the mail was successfully accepted for delivery, false otherwise. + */ + protected function mail(string $to, string $subject, string $body, string $headers, string $parameters) + { + return mail($to, $subject, $body, $headers, $parameters); + } } diff --git a/tests/Util/EmailerSpy.php b/tests/Util/EmailerSpy.php new file mode 100644 index 0000000000..effd2ed080 --- /dev/null +++ b/tests/Util/EmailerSpy.php @@ -0,0 +1,23 @@ + $to, + 'subject' => $subject, + 'body' => $body, + 'headers' => $headers, + 'parameters' => $parameters, + ]; + + return true; + } +} diff --git a/tests/Util/HookMockTrait.php b/tests/Util/HookMockTrait.php new file mode 100644 index 0000000000..68a17c5bdb --- /dev/null +++ b/tests/Util/HookMockTrait.php @@ -0,0 +1,30 @@ +hookMock)) { + $this->hookMock = \Mockery::mock('alias:' . Hook::class); + } + + $this->hookMock + ->shouldReceive('callAll') + ->withArgs([$name, \Mockery::capture($capture)]); + } +} diff --git a/tests/src/Util/EMailerTest.php b/tests/src/Util/EMailerTest.php new file mode 100644 index 0000000000..209195f7e8 --- /dev/null +++ b/tests/src/Util/EMailerTest.php @@ -0,0 +1,133 @@ +setUpVfsDir(); + + $this->config = \Mockery::mock(IConfig::class); + $this->config->shouldReceive('get')->withArgs(['config', 'sender_email'])->andReturn('test@friendica.local')->once(); + $this->config->shouldReceive('get')->withArgs(['config', 'sitename', 'Friendica Social Network'])->andReturn('Friendica Social Network')->once(); + $this->config->shouldReceive('get')->withArgs(['system', 'sendmail_params', true])->andReturn(true); + + $this->pConfig = \Mockery::mock(IPConfig::class); + $this->l10n = \Mockery::mock(L10n::class); + $this->baseUrl = \Mockery::mock(BaseURL::class); + $this->baseUrl->shouldReceive('getHostname')->andReturn('friendica.local'); + $this->baseUrl->shouldReceive('get')->andReturn('http://friendica.local'); + + $this->defaultHeaders = []; + } + + protected function tearDown() + { + EmailerSpy::$MAIL_DATA = []; + + parent::tearDown(); + } + + public function testEmail() + { + $this->pConfig->shouldReceive('get')->withArgs(['1', 'system', 'email_textonly'])->andReturn(false)->once(); + + $builder = new SampleMailBuilder($this->l10n, $this->baseUrl, $this->config, new NullLogger()); + + $testEmail = $builder + ->withRecipient('recipient@friendica.local') + ->withMessage('Test Subject', "Test MessageBold", 'Test Text') + ->withSender('Sender', 'sender@friendica.local') + ->forUser(['uid' => 1]) + ->addHeader('Message-ID', 'first Id') + ->build(true); + + $emailer = new EmailerSpy($this->config, $this->pConfig, $this->baseUrl, new NullLogger(), $this->l10n); + + $this->assertTrue($emailer->send($testEmail)); + + $this->assertContains("X-Friendica-Host : friendica.local", EmailerSpy::$MAIL_DATA['headers']); + $this->assertContains("X-Friendica-Platform : Friendica", EmailerSpy::$MAIL_DATA['headers']); + $this->assertContains("List-ID : ", EmailerSpy::$MAIL_DATA['headers']); + $this->assertContains("List-Archive : ", EmailerSpy::$MAIL_DATA['headers']); + $this->assertContains("Reply-To: Sender ", EmailerSpy::$MAIL_DATA['headers']); + $this->assertContains("MIME-Version: 1.0", EmailerSpy::$MAIL_DATA['headers']); + // Base64 "Test Text" + $this->assertContains(chunk_split(base64_encode('Test Text')), EmailerSpy::$MAIL_DATA['body']); + // Base64 "Test MessageBold" + $this->assertContains(chunk_split(base64_encode("Test MessageBold")), EmailerSpy::$MAIL_DATA['body']); + $this->assertEquals("Test Subject", EmailerSpy::$MAIL_DATA['subject']); + $this->assertEquals("recipient@friendica.local", EmailerSpy::$MAIL_DATA['to']); + $this->assertEquals("-f sender@friendica.local", EmailerSpy::$MAIL_DATA['parameters']); + } + + public function testTwoMessageIds() + { + $this->pConfig->shouldReceive('get')->withArgs(['1', 'system', 'email_textonly'])->andReturn(false)->once(); + + /** @var IEmail $preparedEmail */ + $preparedEmail = null; + /** @var IEmail $sentEMail */ + $sentEMail = null; + + $this->mockHookCallAll('emailer_send_prepare', $preparedEmail); + $this->mockHookCallAll('emailer_send', $sentEMail); + + $builder = new SampleMailBuilder($this->l10n, $this->baseUrl, $this->config, new NullLogger()); + + $testEmail = $builder + ->withRecipient('recipient@friendica.local') + ->withMessage('Test Subject', "Test MessageBold", 'Test Text') + ->withSender('Sender', 'sender@friendica.loca') + ->forUser(['uid' => 1]) + ->addHeader('Message-ID', 'first Id') + ->addHeader('Message-Id', 'second Id') + ->build(true); + + $emailer = new EmailerSpy($this->config, $this->pConfig, $this->baseUrl, new NullLogger(), $this->l10n); + + // even in case there are two message ids, send the mail anyway + $this->assertTrue($emailer->send($testEmail)); + + // check case sensitive key problem + $this->assertArrayHasKey('Message-ID', $testEmail->getAdditionalMailHeader()); + $this->assertArrayHasKey('Message-Id', $testEmail->getAdditionalMailHeader()); + } +} diff --git a/tests/src/Util/Emailer/MailBuilderTest.php b/tests/src/Util/Emailer/MailBuilderTest.php index 202ad587b6..b890224210 100644 --- a/tests/src/Util/Emailer/MailBuilderTest.php +++ b/tests/src/Util/Emailer/MailBuilderTest.php @@ -29,6 +29,7 @@ use Friendica\Test\MockedTest; use Friendica\Test\Util\SampleMailBuilder; use Friendica\Test\Util\VFSTrait; use Friendica\Util\EMailer\MailBuilder; +use Mockery\MockInterface; use Psr\Log\NullLogger; /** @@ -39,11 +40,11 @@ class MailBuilderTest extends MockedTest { use VFSTrait; - /** @var IConfig */ + /** @var IConfig|MockInterface */ private $config; - /** @var L10n */ + /** @var L10n|MockInterface */ private $l10n; - /** @var BaseURL */ + /** @var BaseURL|MockInterface */ private $baseUrl; /** @var string */