Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion 3rdparty
Submodule 3rdparty updated 186 files
11 changes: 3 additions & 8 deletions config/config.sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -328,24 +328,19 @@
'mail_smtpdebug' => false,

/**
* Which mode to use for sending mail: ``sendmail``, ``smtp``, ``qmail`` or
* ``php``.
* Which mode to use for sending mail: ``sendmail``, ``smtp`` or ``qmail``.
*
* If you are using local or remote SMTP, set this to ``smtp``.
*
* If you are using PHP mail you must have an installed and working email system
* on the server. The program used to send email is defined in the ``php.ini``
* file.
*
* For the ``sendmail`` option you need an installed and working email system on
* the server, with ``/usr/sbin/sendmail`` installed on your Unix system.
*
* For ``qmail`` the binary is /var/qmail/bin/sendmail, and it must be installed
* on your Unix system.
*
* Defaults to ``php``
* Defaults to ``smtp``
*/
'mail_smtpmode' => 'php',
'mail_smtpmode' => 'smtp',

/**
* This depends on ``mail_smtpmode``. Specify the IP address of your mail
Expand Down
12 changes: 12 additions & 0 deletions core/js/setupchecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,18 @@
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
})
}
if (data.isPhpMailerUsed) {
messages.push({
msg: t(
'core',
'Use of the the built in php mailer is no longer supported. <a target="_blank" rel="noreferrer noopener" href="{docLink}">Please update your email server settings ↗<a/>.',
{
docLink: data.mailSettingsDocumentation,
}
),
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
});
}
} else {
messages.push({
msg: t('core', 'Error occurred while checking server setup'),
Expand Down
53 changes: 20 additions & 33 deletions lib/private/Mail/Mailer.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

namespace OC\Mail;

use Egulias\EmailValidator\EmailValidator;
use Egulias\EmailValidator\Validation\RFCValidation;
use OCP\Defaults;
use OCP\IConfig;
use OCP\IL10N;
Expand Down Expand Up @@ -55,7 +57,7 @@
* @package OC\Mail
*/
class Mailer implements IMailer {
/** @var \Swift_SmtpTransport|\Swift_SendmailTransport|\Swift_MailTransport Cached transport */
/** @var \Swift_Mailer Cached mailer */
private $instance = null;
/** @var IConfig */
private $config;
Expand Down Expand Up @@ -105,7 +107,7 @@ public function createMessage(): IMessage {
* @since 13.0.0
*/
public function createAttachment($data = null, $filename = null, $contentType = null): IAttachment {
return new Attachment(\Swift_Attachment::newInstance($data, $filename, $contentType));
return new Attachment(new \Swift_Attachment($data, $filename, $contentType));
}

/**
Expand Down Expand Up @@ -194,7 +196,10 @@ public function send(IMessage $message): array {
* @return bool True if the mail address is valid, false otherwise
*/
public function validateMailAddress(string $email): bool {
return \Swift_Validate::email($this->convertEmail($email));
$validator = new EmailValidator();
$validation = new RFCValidation();

return $validator->isValid($this->convertEmail($email), $validation);
}

/**
Expand All @@ -215,32 +220,24 @@ protected function convertEmail(string $email): string {
return $name.'@'.$domain;
}

/**
* Returns whatever transport is configured within the config
*
* @return \Swift_SmtpTransport|\Swift_SendmailTransport|\Swift_MailTransport
*/
protected function getInstance() {
protected function getInstance(): \Swift_Mailer {
if (!is_null($this->instance)) {
return $this->instance;
}

switch ($this->config->getSystemValue('mail_smtpmode', 'php')) {
case 'smtp':
$this->instance = $this->getSmtpInstance();
break;
$transport = null;

switch ($this->config->getSystemValue('mail_smtpmode', 'smtp')) {
case 'sendmail':
// FIXME: Move into the return statement but requires proper testing
// for SMTP and mail as well. Thus not really doable for a
// minor release.
$this->instance = \Swift_Mailer::newInstance($this->getSendMailInstance());
$transport = $this->getSendMailInstance();
break;
case 'smtp':
default:
$this->instance = $this->getMailInstance();
$transport = $this->getSmtpInstance();
break;
}

return $this->instance;
return new \Swift_Mailer($transport);
}

/**
Expand All @@ -249,7 +246,7 @@ protected function getInstance() {
* @return \Swift_SmtpTransport
*/
protected function getSmtpInstance(): \Swift_SmtpTransport {
$transport = \Swift_SmtpTransport::newInstance();
$transport = new \Swift_SmtpTransport();
$transport->setTimeout($this->config->getSystemValue('mail_smtptimeout', 10));
$transport->setHost($this->config->getSystemValue('mail_smtphost', '127.0.0.1'));
$transport->setPort($this->config->getSystemValue('mail_smtpport', 25));
Expand All @@ -262,7 +259,7 @@ protected function getSmtpInstance(): \Swift_SmtpTransport {
if (!empty($smtpSecurity)) {
$transport->setEncryption($smtpSecurity);
}
$transport->start();

return $transport;
}

Expand All @@ -272,7 +269,7 @@ protected function getSmtpInstance(): \Swift_SmtpTransport {
* @return \Swift_SendmailTransport
*/
protected function getSendMailInstance(): \Swift_SendmailTransport {
switch ($this->config->getSystemValue('mail_smtpmode', 'php')) {
switch ($this->config->getSystemValue('mail_smtpmode', 'smtp')) {
case 'qmail':
$binaryPath = '/var/qmail/bin/sendmail';
break;
Expand All @@ -281,16 +278,6 @@ protected function getSendMailInstance(): \Swift_SendmailTransport {
break;
}

return \Swift_SendmailTransport::newInstance($binaryPath . ' -bs');
return new \Swift_SendmailTransport($binaryPath . ' -bs');
}

/**
* Returns the mail transport
*
* @return \Swift_MailTransport
*/
protected function getMailInstance(): \Swift_MailTransport {
return \Swift_MailTransport::newInstance();
}

}
4 changes: 4 additions & 0 deletions lib/private/Settings/Admin/Mail.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ public function getForm() {
$parameters['mail_smtppassword'] = '********';
}

if ($parameters['mail_smtpmode'] === '' || $parameters['mail_smtpmode'] === 'php') {
$parameters['mail_smtpmode'] = 'smtp';
}

return new TemplateResponse('settings', 'settings/admin/additional-mail', $parameters, '');
}

Expand Down
6 changes: 6 additions & 0 deletions settings/Controller/CheckSetupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,10 @@ protected function getCronErrors() {
return [];
}

protected function isPhpMailerUsed(): bool {
return $this->config->getSystemValue('mail_smtpmode', 'php') === 'php';
}

/**
* @return DataResponse
*/
Expand Down Expand Up @@ -557,6 +561,8 @@ public function check() {
'missingIndexes' => $this->hasMissingIndexes(),
'isSqliteUsed' => $this->isSqliteUsed(),
'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'),
'isPhpMailerUsed' => $this->isPhpMailerUsed(),
'mailSettingsDocumentation' => $this->urlGenerator->getAbsoluteURL('index.php/settings/admin')
]
);
}
Expand Down
1 change: 0 additions & 1 deletion settings/templates/settings/admin/additional-mail.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
];

$mail_smtpmode = [
['php', 'PHP'],
['smtp', 'SMTP'],
];
if ($_['sendmail_is_available']) {
Expand Down
27 changes: 26 additions & 1 deletion tests/Settings/Controller/CheckSetupControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,22 @@ public function setUp() {
$this->lockingProvider,
$this->dateTimeFormatter,
])
->setMethods(['isReadOnlyConfig', 'hasValidTransactionIsolationLevel', 'hasFileinfoInstalled', 'hasWorkingFileLocking', 'getLastCronInfo', 'getSuggestedOverwriteCliURL', 'getOutdatedCaches', 'getCurlVersion', 'isPhpOutdated', 'isOpcacheProperlySetup', 'hasFreeTypeSupport', 'hasMissingIndexes', 'isSqliteUsed'])->getMock();
->setMethods([
'isReadOnlyConfig',
'hasValidTransactionIsolationLevel',
'hasFileinfoInstalled',
'hasWorkingFileLocking',
'getLastCronInfo',
'getSuggestedOverwriteCliURL',
'getOutdatedCaches',
'getCurlVersion',
'isPhpOutdated',
'isOpcacheProperlySetup',
'hasFreeTypeSupport',
'hasMissingIndexes',
'isSqliteUsed',
'isPhpMailerUsed',
])->getMock();
}

public function testIsInternetConnectionWorkingDisabledViaConfig() {
Expand Down Expand Up @@ -352,6 +367,10 @@ public function testCheck() {
->method('linkToDocs')
->with('admin-db-conversion')
->willReturn('http://docs.example.org/server/go.php?to=admin-db-conversion');
$this->urlGenerator->expects($this->at(6))
->method('getAbsoluteURL')
->with('index.php/settings/admin')
->willReturn('https://server/index.php/settings/admin');
$this->checkSetupController
->method('hasFreeTypeSupport')
->willReturn(false);
Expand Down Expand Up @@ -392,6 +411,10 @@ public function testCheck() {
'relativeTime' => '2 hours ago',
'backgroundJobsUrl' => 'https://example.org',
]);
$this->checkSetupController
->expects($this->once())
->method('isPhpMailerUsed')
->willReturn(false);
$this->checker
->expects($this->once())
->method('hasPassedCheck')
Expand Down Expand Up @@ -434,6 +457,8 @@ public function testCheck() {
'isSqliteUsed' => false,
'databaseConversionDocumentation' => 'http://docs.example.org/server/go.php?to=admin-db-conversion',
'missingIndexes' => [],
'isPhpMailerUsed' => false,
'mailSettingsDocumentation' => 'https://server/index.php/settings/admin',
]
);
$this->assertEquals($expected, $this->checkSetupController->check());
Expand Down
33 changes: 12 additions & 21 deletions tests/lib/Mail/MailerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,50 +48,41 @@ public function setUp() {
);
}

public function testGetMailInstance() {
$this->assertEquals(\Swift_MailTransport::newInstance(), self::invokePrivate($this->mailer, 'getMailinstance'));
}

public function testGetSendMailInstanceSendMail() {
$this->config
->expects($this->once())
->method('getSystemValue')
->with('mail_smtpmode', 'php')
->with('mail_smtpmode', 'smtp')
->will($this->returnValue('sendmail'));

$this->assertEquals(\Swift_SendmailTransport::newInstance('/usr/sbin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance'));
$this->assertEquals(new \Swift_SendmailTransport('/usr/sbin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance'));
}

public function testGetSendMailInstanceSendMailQmail() {
$this->config
->expects($this->once())
->method('getSystemValue')
->with('mail_smtpmode', 'php')
->with('mail_smtpmode', 'smtp')
->will($this->returnValue('qmail'));

$this->assertEquals(\Swift_SendmailTransport::newInstance('/var/qmail/bin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance'));
$this->assertEquals(new \Swift_SendmailTransport('/var/qmail/bin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance'));
}

public function testGetInstanceDefault() {
$this->assertInstanceOf('\Swift_MailTransport', self::invokePrivate($this->mailer, 'getInstance'));
}

public function testGetInstancePhp() {
$this->config
->expects($this->any())
->method('getSystemValue')
->will($this->returnValue('php'));

$this->assertInstanceOf('\Swift_MailTransport', self::invokePrivate($this->mailer, 'getInstance'));
$mailer = self::invokePrivate($this->mailer, 'getInstance');
$this->assertInstanceOf(\Swift_Mailer::class, $mailer);
$this->assertInstanceOf(\Swift_SmtpTransport::class, $mailer->getTransport());
}

public function testGetInstanceSendmail() {
$this->config
->expects($this->any())
->method('getSystemValue')
->will($this->returnValue('sendmail'));
->with('mail_smtpmode', 'smtp')
->willReturn('sendmail');

$this->assertInstanceOf('\Swift_Mailer', self::invokePrivate($this->mailer, 'getInstance'));
$mailer = self::invokePrivate($this->mailer, 'getInstance');
$this->assertInstanceOf(\Swift_Mailer::class, $mailer);
$this->assertInstanceOf(\Swift_SendmailTransport::class, $mailer->getTransport());
}

public function testCreateMessage() {
Expand Down
4 changes: 2 additions & 2 deletions tests/lib/Settings/Admin/MailTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function testGetForm() {
->expects($this->at(2))
->method('getSystemValue')
->with('mail_smtpmode', '')
->willReturn('php');
->willReturn('smtp');
$this->config
->expects($this->at(3))
->method('getSystemValue')
Expand Down Expand Up @@ -103,7 +103,7 @@ public function testGetForm() {
'sendmail_is_available' => (bool) \OC_Helper::findBinaryPath('sendmail'),
'mail_domain' => 'mx.nextcloud.com',
'mail_from_address' => 'no-reply@nextcloud.com',
'mail_smtpmode' => 'php',
'mail_smtpmode' => 'smtp',
'mail_smtpsecure' => true,
'mail_smtphost' => 'smtp.nextcloud.com',
'mail_smtpport' => 25,
Expand Down