Fix idn emails not working in shares

And add check before sending email that email address is valid

Fix #30595

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

2 files changed, 19 insertions, 5 deletions

diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php
index 021f96020c5..fb8bee3f7fd 100644
--- a/apps/sharebymail/lib/ShareByMailProvider.php
+++ b/apps/sharebymail/lib/ShareByMailProvider.php
@@ -325,6 +325,16 @@ class ShareByMailProvider implements IShareProvider {
 			$share->getExpirationDate()
 		);
 
+		if (!$this->mailer->validateMailAddress($share->getSharedWith())) {
+			$this->removeShareFromTable($shareId);
+			$e = new HintException('Failed to send share by mail. Got an invalid email address: ' . $share->getSharedWith(),
+				$this->l->t('Failed to send share by email. Got an invalid email address'));
+			$this->logger->error('Failed to send share by mail. Got an invalid email address ' . $share->getSharedWith(), [
+				'app' => 'sharebymail',
+				'exception' => $e,
+			]);
+		}
+
 		try {
 			$link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare',
 				['token' => $share->getToken()]);
@@ -653,7 +663,7 @@ class ShareByMailProvider implements IShareProvider {
 	 * @param \DateTime|null $expirationTime
 	 * @return int
 	 */
-	protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $sendPasswordByTalk, $hideDownload, $expirationTime) {
+	protected function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $password, $sendPasswordByTalk, $hideDownload, $expirationTime): int {
 		$qb = $this->dbConnection->getQueryBuilder();
 		$qb->insert('share')
 			->setValue('share_type', $qb->createNamedParameter(IShare::TYPE_EMAIL))
@@ -747,7 +757,7 @@ class ShareByMailProvider implements IShareProvider {
 		} catch (\Exception $e) {
 		}
 
-		$this->removeShareFromTable($share->getId());
+		$this->removeShareFromTable((int)$share->getId());
 	}
 
 	/**
@@ -943,9 +953,9 @@ class ShareByMailProvider implements IShareProvider {
 	/**
 	 * remove share from table
 	 *
-	 * @param string $shareId
+	 * @param int $shareId
 	 */
-	protected function removeShareFromTable($shareId) {
+	protected function removeShareFromTable(int $shareId): void {
 		$qb = $this->dbConnection->getQueryBuilder();
 		$qb->delete('share')
 			->where($qb->expr()->eq('id', $qb->createNamedParameter($shareId)));
diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php
index 4989f33d0e2..57cd92d5dea 100644
--- a/apps/sharebymail/tests/ShareByMailProviderTest.php
+++ b/apps/sharebymail/tests/ShareByMailProviderTest.php
@@ -214,7 +214,7 @@ class ShareByMailProviderTest extends TestCase {
 
 	public function testCreateSendPasswordByMailWithoutEnforcedPasswordProtection() {
 		$share = $this->getMockBuilder(IShare::class)->getMock();
-		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
+		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@examplelölöl.com');
 		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
 		$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
 
@@ -456,6 +456,7 @@ class ShareByMailProviderTest extends TestCase {
 	public function testCreateMailShare() {
 		$this->share->expects($this->any())->method('getToken')->willReturn('token');
 		$this->share->expects($this->once())->method('setToken')->with('token');
+		$this->share->expects($this->any())->method('getSharedWith')->willReturn('valid@valid.com');
 		$node = $this->getMockBuilder('OCP\Files\Node')->getMock();
 		$node->expects($this->any())->method('getName')->willReturn('fileName');
 		$this->share->expects($this->any())->method('getNode')->willReturn($node);
@@ -480,6 +481,7 @@ class ShareByMailProviderTest extends TestCase {
 
 		$this->share->expects($this->any())->method('getToken')->willReturn('token');
 		$this->share->expects($this->once())->method('setToken')->with('token');
+		$this->share->expects($this->any())->method('getSharedWith')->willReturn('valid@valid.com');
 		$node = $this->getMockBuilder('OCP\Files\Node')->getMock();
 		$node->expects($this->any())->method('getName')->willReturn('fileName');
 		$this->share->expects($this->any())->method('getNode')->willReturn($node);
@@ -977,6 +979,7 @@ class ShareByMailProviderTest extends TestCase {
 		$rootFolder = \OC::$server->getRootFolder();
 
 		$provider = $this->getInstance(['sendMailNotification', 'createShareActivity']);
+		$this->mailer->expects($this->any())->method('validateMailAddress')->willReturn(true);
 
 		$u1 = $userManager->createUser('testFed', md5(time()));
 		$u2 = $userManager->createUser('testFed2', md5(time()));
@@ -1019,6 +1022,7 @@ class ShareByMailProviderTest extends TestCase {
 		$rootFolder = \OC::$server->getRootFolder();
 
 		$provider = $this->getInstance(['sendMailNotification', 'createShareActivity']);
+		$this->mailer->expects($this->any())->method('validateMailAddress')->willReturn(true);
 
 		$u1 = $userManager->createUser('testFed', md5(time()));
 		$u2 = $userManager->createUser('testFed2', md5(time()));