Merge pull request #9791 from nextcloud/3rdparty/noid/bump_swiftmailer

Upgrade to swiftmailer-6

10 files changed, 85 insertions, 66 deletions

diff --git a/3rdparty b/3rdparty
-Subproject 7d055b064564a2bc5c4109860b12b6b30202f50
+Subproject b995ca8b8c7f69a180ad6fb49989ad3e35b1367
diff --git a/config/config.sample.php b/config/config.sample.php
index 68f27ed0323..5c6ca58a640 100644
--- a/config/config.sample.php
+++ b/config/config.sample.php
@@ -328,24 +328,19 @@ $CONFIG = array(
 '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
diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js
index eae0abae50c..75d335043ac 100644
--- a/core/js/setupchecks.js
+++ b/core/js/setupchecks.js
@@ -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'),
diff --git a/lib/private/Mail/Mailer.php b/lib/private/Mail/Mailer.php
index 001f7bd75c9..6f148bc0c6e 100644
--- a/lib/private/Mail/Mailer.php
+++ b/lib/private/Mail/Mailer.php
@@ -26,6 +26,8 @@ declare(strict_types=1);
 
 namespace OC\Mail;
 
+use Egulias\EmailValidator\EmailValidator;
+use Egulias\EmailValidator\Validation\RFCValidation;
 use OCP\Defaults;
 use OCP\IConfig;
 use OCP\IL10N;
@@ -55,7 +57,7 @@ use OCP\Mail\IMessage;
  * @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;
@@ -105,7 +107,7 @@ class Mailer implements IMailer {
 	 * @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));
 	}
 
 	/**
@@ -194,7 +196,10 @@ class Mailer implements IMailer {
 	 * @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);
 	}
 
 	/**
@@ -215,32 +220,24 @@ class Mailer implements IMailer {
 		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);
 	}
 
 	/**
@@ -249,7 +246,7 @@ class Mailer implements IMailer {
 	 * @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));
@@ -262,7 +259,7 @@ class Mailer implements IMailer {
 		if (!empty($smtpSecurity)) {
 			$transport->setEncryption($smtpSecurity);
 		}
-		$transport->start();
+
 		return $transport;
 	}
 
@@ -272,7 +269,7 @@ class Mailer implements IMailer {
 	 * @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;
@@ -281,16 +278,6 @@ class Mailer implements IMailer {
 				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();
-	}
-
 }
diff --git a/lib/private/Settings/Admin/Mail.php b/lib/private/Settings/Admin/Mail.php
index 74a94a4c7a0..fc20b7eeb3f 100644
--- a/lib/private/Settings/Admin/Mail.php
+++ b/lib/private/Settings/Admin/Mail.php
@@ -63,6 +63,10 @@ class Mail implements ISettings {
 			$parameters['mail_smtppassword'] = '********';
 		}
 
+		if ($parameters['mail_smtpmode'] === '' || $parameters['mail_smtpmode'] === 'php') {
+			$parameters['mail_smtpmode'] = 'smtp';
+		}
+
 		return new TemplateResponse('settings', 'settings/admin/additional-mail', $parameters, '');
 	}
 
diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php
index ecbb9839c75..a301ecb1f66 100644
--- a/settings/Controller/CheckSetupController.php
+++ b/settings/Controller/CheckSetupController.php
@@ -523,6 +523,10 @@ Raw output
 		return [];
 	}
 
+	protected function isPhpMailerUsed(): bool {
+		return $this->config->getSystemValue('mail_smtpmode', 'php') === 'php';
+	}
+
 	/**
 	 * @return DataResponse
 	 */
@@ -557,6 +561,8 @@ Raw output
 				'missingIndexes' => $this->hasMissingIndexes(),
 				'isSqliteUsed' => $this->isSqliteUsed(),
 				'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'),
+				'isPhpMailerUsed' => $this->isPhpMailerUsed(),
+				'mailSettingsDocumentation' => $this->urlGenerator->getAbsoluteURL('index.php/settings/admin')
 			]
 		);
 	}
diff --git a/settings/templates/settings/admin/additional-mail.php b/settings/templates/settings/admin/additional-mail.php
index adcc5293ff1..1e0b458f60f 100644
--- a/settings/templates/settings/admin/additional-mail.php
+++ b/settings/templates/settings/admin/additional-mail.php
@@ -38,7 +38,6 @@ $mail_smtpsecure = [
 ];
 
 $mail_smtpmode = [
-	['php', 'PHP'],
 	['smtp', 'SMTP'],
 ];
 if ($_['sendmail_is_available']) {
diff --git a/tests/Settings/Controller/CheckSetupControllerTest.php b/tests/Settings/Controller/CheckSetupControllerTest.php
index 470bc9cde64..057774a45ba 100644
--- a/tests/Settings/Controller/CheckSetupControllerTest.php
+++ b/tests/Settings/Controller/CheckSetupControllerTest.php
@@ -119,7 +119,22 @@ class CheckSetupControllerTest extends TestCase {
 				$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() {
@@ -352,6 +367,10 @@ class CheckSetupControllerTest extends TestCase {
 			->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);
@@ -392,6 +411,10 @@ class CheckSetupControllerTest extends TestCase {
 				'relativeTime' => '2 hours ago',
 				'backgroundJobsUrl' => 'https://example.org',
 			]);
+		$this->checkSetupController
+			->expects($this->once())
+			->method('isPhpMailerUsed')
+			->willReturn(false);
 		$this->checker
 			->expects($this->once())
 			->method('hasPassedCheck')
@@ -434,6 +457,8 @@ class CheckSetupControllerTest extends TestCase {
 				'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());
diff --git a/tests/lib/Mail/MailerTest.php b/tests/lib/Mail/MailerTest.php
index 2dd4bca5190..d724cd630d3 100644
--- a/tests/lib/Mail/MailerTest.php
+++ b/tests/lib/Mail/MailerTest.php
@@ -48,50 +48,41 @@ class MailerTest extends TestCase {
 		);
 	}
 
-	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() {
diff --git a/tests/lib/Settings/Admin/MailTest.php b/tests/lib/Settings/Admin/MailTest.php
index 04c0e7fa92e..436a7953220 100644
--- a/tests/lib/Settings/Admin/MailTest.php
+++ b/tests/lib/Settings/Admin/MailTest.php
@@ -59,7 +59,7 @@ class MailTest extends TestCase {
 			->expects($this->at(2))
 			->method('getSystemValue')
 			->with('mail_smtpmode', '')
-			->willReturn('php');
+			->willReturn('smtp');
 		$this->config
 			->expects($this->at(3))
 			->method('getSystemValue')
@@ -103,7 +103,7 @@ class MailTest extends TestCase {
 				'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,