From 571e3c155cc97e76a654222f53187b9ca2e48be4 Mon Sep 17 00:00:00 2001 From: Carsten Wiedmann Date: Mon, 12 Nov 2018 23:26:35 +0100 Subject: [PATCH] Apply patch from @cwiedmann but drop -oi option for pipe * Add sendmailmode to gui * Add testcases for pipe mode Signed-off-by: Daniel Kesselberg --- config/config.sample.php | 13 +++++ lib/private/Mail/Mailer.php | 11 +++- lib/private/Settings/Admin/Mail.php | 1 + .../Controller/MailSettingsController.php | 3 +- settings/js/admin.js | 2 + .../settings/admin/additional-mail.php | 14 +++++ .../Controller/MailSettingsControllerTest.php | 8 ++- tests/lib/Mail/MailerTest.php | 56 ++++++++++++++----- tests/lib/Settings/Admin/MailTest.php | 6 ++ 9 files changed, 95 insertions(+), 19 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index 874fbc04e5..f12c75d91c 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -421,6 +421,19 @@ $CONFIG = array( */ 'mail_send_plaintext_only' => false, +/** + * Which mode is used for sendmail/qmail: ``smtp`` or ``pipe``. + * + * For ``smtp`` the sendmail binary is started with the parameter ``-bs``: + * - Use the SMTP protocol on standard input and output. + * + * For ``pipe`` the binary is started with the parameters ``-t``: + * - Read message from STDIN and extract recipients. + * + * Defaults to ``smtp`` + */ +'mail_sendmailmode' => 'smtp', + /** * Proxy Configurations */ diff --git a/lib/private/Mail/Mailer.php b/lib/private/Mail/Mailer.php index 6f148bc0c6..e39c88677f 100644 --- a/lib/private/Mail/Mailer.php +++ b/lib/private/Mail/Mailer.php @@ -278,6 +278,15 @@ class Mailer implements IMailer { break; } - return new \Swift_SendmailTransport($binaryPath . ' -bs'); + switch ($this->config->getSystemValue('mail_sendmailmode', 'smtp')) { + case 'pipe': + $binaryParam = ' -t'; + break; + default: + $binaryParam = ' -bs'; + break; + } + + return new \Swift_SendmailTransport($binaryPath . $binaryParam); } } diff --git a/lib/private/Settings/Admin/Mail.php b/lib/private/Settings/Admin/Mail.php index fc20b7eeb3..40dcc3dba0 100644 --- a/lib/private/Settings/Admin/Mail.php +++ b/lib/private/Settings/Admin/Mail.php @@ -57,6 +57,7 @@ class Mail implements ISettings { 'mail_smtpauth' => $this->config->getSystemValue('mail_smtpauth', false), 'mail_smtpname' => $this->config->getSystemValue('mail_smtpname', ''), 'mail_smtppassword' => $this->config->getSystemValue('mail_smtppassword', ''), + 'mail_sendmailmode' => $this->config->getSystemValue('mail_sendmailmode', 'smtp'), ]; if ($parameters['mail_smtppassword'] !== '') { diff --git a/settings/Controller/MailSettingsController.php b/settings/Controller/MailSettingsController.php index d1ceb14a63..5d2e67ce6e 100644 --- a/settings/Controller/MailSettingsController.php +++ b/settings/Controller/MailSettingsController.php @@ -91,7 +91,8 @@ class MailSettingsController extends Controller { $mail_smtphost, $mail_smtpauthtype, $mail_smtpauth, - $mail_smtpport) { + $mail_smtpport, + $mail_sendmailmode) { $params = get_defined_vars(); $configs = []; diff --git a/settings/js/admin.js b/settings/js/admin.js index de5bc2b953..27e5b2d12d 100644 --- a/settings/js/admin.js +++ b/settings/js/admin.js @@ -161,6 +161,7 @@ $(document).ready(function(){ $('#mail_smtpsecure_label').addClass('hidden'); $('#mail_smtpsecure').addClass('hidden'); $('#mail_credentials').addClass('hidden'); + $('#mail_sendmailmode_label, #mail_sendmailmode').removeClass('hidden'); } else { $('#setting_smtpauth').removeClass('hidden'); $('#setting_smtphost').removeClass('hidden'); @@ -169,6 +170,7 @@ $(document).ready(function(){ if ($('#mail_smtpauth').is(':checked')) { $('#mail_credentials').removeClass('hidden'); } + $('#mail_sendmailmode_label, #mail_sendmailmode').addClass('hidden'); } }); diff --git a/settings/templates/settings/admin/additional-mail.php b/settings/templates/settings/admin/additional-mail.php index 0a70801596..362c301281 100644 --- a/settings/templates/settings/admin/additional-mail.php +++ b/settings/templates/settings/admin/additional-mail.php @@ -47,6 +47,11 @@ if ($_['mail_smtpmode'] === 'qmail') { $mail_smtpmode[] = ['qmail', 'qmail']; } +$mail_sendmailmode = [ + 'smtp' => 'smtp (-bs)', + 'pipe' => 'pipe (-t)' +]; + ?>
@@ -84,6 +89,15 @@ if ($_['mail_smtpmode'] === 'qmail') { + + +

diff --git a/tests/Settings/Controller/MailSettingsControllerTest.php b/tests/Settings/Controller/MailSettingsControllerTest.php index 79fe0683cc..ed241ed053 100644 --- a/tests/Settings/Controller/MailSettingsControllerTest.php +++ b/tests/Settings/Controller/MailSettingsControllerTest.php @@ -74,6 +74,7 @@ class MailSettingsControllerTest extends \Test\TestCase { 'mail_smtpauthtype' => 'NTLM', 'mail_smtpauth' => 1, 'mail_smtpport' => '25', + 'mail_sendmailmode' => null, ]], [[ 'mail_domain' => 'nextcloud.com', @@ -86,6 +87,7 @@ class MailSettingsControllerTest extends \Test\TestCase { 'mail_smtpport' => '25', 'mail_smtpname' => null, 'mail_smtppassword' => null, + 'mail_sendmailmode' => null, ]] ); @@ -98,7 +100,8 @@ class MailSettingsControllerTest extends \Test\TestCase { 'mx.nextcloud.org', 'NTLM', 1, - '25' + '25', + null ); $this->assertSame(Http::STATUS_OK, $response->getStatus()); @@ -111,7 +114,8 @@ class MailSettingsControllerTest extends \Test\TestCase { 'mx.nextcloud.org', 'NTLM', 0, - '25' + '25', + null ); $this->assertSame(Http::STATUS_OK, $response->getStatus()); diff --git a/tests/lib/Mail/MailerTest.php b/tests/lib/Mail/MailerTest.php index d724cd630d..aac88728c3 100644 --- a/tests/lib/Mail/MailerTest.php +++ b/tests/lib/Mail/MailerTest.php @@ -48,24 +48,48 @@ class MailerTest extends TestCase { ); } - public function testGetSendMailInstanceSendMail() { - $this->config - ->expects($this->once()) - ->method('getSystemValue') - ->with('mail_smtpmode', 'smtp') - ->will($this->returnValue('sendmail')); - - $this->assertEquals(new \Swift_SendmailTransport('/usr/sbin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance')); + /** + * @return array + */ + public function sendmailModeProvider(): array { + return [ + 'smtp' => ['smtp', ' -bs'], + 'pipe' => ['pipe', ' -t'], + ]; } - public function testGetSendMailInstanceSendMailQmail() { + /** + * @dataProvider sendmailModeProvider + * @param $sendmailMode + * @param $binaryParam + */ + public function testGetSendmailInstanceSendMail($sendmailMode, $binaryParam) { $this->config - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getSystemValue') - ->with('mail_smtpmode', 'smtp') - ->will($this->returnValue('qmail')); + ->will($this->returnValueMap([ + ['mail_smtpmode', 'smtp', 'sendmail'], + ['mail_sendmailmode', 'smtp', $sendmailMode], + ])); - $this->assertEquals(new \Swift_SendmailTransport('/var/qmail/bin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance')); + $this->assertEquals(new \Swift_SendmailTransport('/usr/sbin/sendmail' . $binaryParam), self::invokePrivate($this->mailer, 'getSendMailInstance')); + } + + /** + * @dataProvider sendmailModeProvider + * @param $sendmailMode + * @param $binaryParam + */ + public function testGetSendmailInstanceSendMailQmail($sendmailMode, $binaryParam) { + $this->config + ->expects($this->exactly(2)) + ->method('getSystemValue') + ->will($this->returnValueMap([ + ['mail_smtpmode', 'smtp', 'qmail'], + ['mail_sendmailmode', 'smtp', $sendmailMode], + ])); + + $this->assertEquals(new \Swift_SendmailTransport('/var/qmail/bin/sendmail' . $binaryParam), self::invokePrivate($this->mailer, 'getSendMailInstance')); } public function testGetInstanceDefault() { @@ -77,8 +101,10 @@ class MailerTest extends TestCase { public function testGetInstanceSendmail() { $this->config ->method('getSystemValue') - ->with('mail_smtpmode', 'smtp') - ->willReturn('sendmail'); + ->will($this->returnValueMap([ + ['mail_smtpmode', 'smtp', 'sendmail'], + ['mail_sendmailmode', 'smtp', 'smtp'], + ])); $mailer = self::invokePrivate($this->mailer, 'getInstance'); $this->assertInstanceOf(\Swift_Mailer::class, $mailer); diff --git a/tests/lib/Settings/Admin/MailTest.php b/tests/lib/Settings/Admin/MailTest.php index 436a795322..1a1d090418 100644 --- a/tests/lib/Settings/Admin/MailTest.php +++ b/tests/lib/Settings/Admin/MailTest.php @@ -95,6 +95,11 @@ class MailTest extends TestCase { ->method('getSystemValue') ->with('mail_smtppassword', '') ->willReturn('mypassword'); + $this->config + ->expects($this->at(10)) + ->method('getSystemValue') + ->with('mail_sendmailmode', 'smtp') + ->willReturn('smtp'); $expected = new TemplateResponse( 'settings', @@ -111,6 +116,7 @@ class MailTest extends TestCase { 'mail_smtpauth' => true, 'mail_smtpname' => 'smtp.sender.com', 'mail_smtppassword' => '********', + 'mail_sendmailmode' => 'smtp', ], '' );