Merge pull request #12129 from nextcloud/fix/imip-processing-error-catch

fix: handle imip failures properly

Author: ChristophWurst

lib/Service/IMipService.php

@@ -19,6 +19,8 @@
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\Calendar\IManager;
 use Psr\Log\LoggerInterface;
+use Throwable;
+
 use function array_filter;
 
 class IMipService {
@@ -121,31 +123,44 @@ public function process(): void {
 				$imapMessage = current(array_filter($imapMessages, static fn (IMAPMessage $imapMessage) => $message->getUid() === $imapMessage->getUid()));
 				if (empty($imapMessage->scheduling)) {
 					// No scheduling info, maybe the DB is wrong
+					$message->setImipProcessed(true);
 					$message->setImipError(true);
 					continue;
 				}
 
 				$sender = $imapMessage->getFrom()->first()?->getEmail();
 				if ($sender === null) {
+					$message->setImipProcessed(true);
 					$message->setImipError(true);
 					continue;
 				}
 
-				foreach ($imapMessage->scheduling as $schedulingInfo) { // an IMAP message could contain more than one iMIP object
-					if ($schedulingInfo['method'] === 'REQUEST') {
-						$processed = $this->calendarManager->handleIMipRequest($principalUri, $sender, $recipient, $schedulingInfo['contents']);
-						$message->setImipProcessed($processed);
-						$message->setImipError(!$processed);
-					} elseif ($schedulingInfo['method'] === 'REPLY') {
-						$processed = $this->calendarManager->handleIMipReply($principalUri, $sender, $recipient, $schedulingInfo['contents']);
-						$message->setImipProcessed($processed);
-						$message->setImipError(!$processed);
-					} elseif ($schedulingInfo['method'] === 'CANCEL') {
-						$replyTo = $imapMessage->getReplyTo()->first()?->getEmail();
-						$processed = $this->calendarManager->handleIMipCancel($principalUri, $sender, $replyTo, $recipient, $schedulingInfo['contents']);
-						$message->setImipProcessed($processed);
-						$message->setImipError(!$processed);
+				try {
+					// an IMAP message could contain more than one iMIP object
+					foreach ($imapMessage->scheduling as $schedulingInfo) {
+						if ($schedulingInfo['method'] === 'REQUEST') {
+							$processed = $this->calendarManager->handleIMipRequest($principalUri, $sender, $recipient, $schedulingInfo['contents']);
+							$message->setImipProcessed($processed);
+							$message->setImipError(!$processed);
+						} elseif ($schedulingInfo['method'] === 'REPLY') {
+							$processed = $this->calendarManager->handleIMipReply($principalUri, $sender, $recipient, $schedulingInfo['contents']);
+							$message->setImipProcessed($processed);
+							$message->setImipError(!$processed);
+						} elseif ($schedulingInfo['method'] === 'CANCEL') {
+							$replyTo = $imapMessage->getReplyTo()->first()?->getEmail();
+							$processed = $this->calendarManager->handleIMipCancel($principalUri, $sender, $replyTo, $recipient, $schedulingInfo['contents']);
+							$message->setImipProcessed($processed);
+							$message->setImipError(!$processed);
+						}
 					}
+				} catch (Throwable $e) {
+					$this->logger->error('iMIP message processing failed', [
+						'exception' => $e,
+						'messageId' => $message->getId(),
+						'mailboxId' => $mailbox->getId(),
+					]);
+					$message->setImipProcessed(true);
+					$message->setImipError(true);
 				}
 			}
 			$this->messageMapper->updateImipData(...$filteredMessages);

tests/Unit/Service/IMipServiceTest.php

@@ -413,4 +413,69 @@ public function testIsCancel(): void {
 
 		$this->service->process();
 	}
+
+	public function testHandleImipRequestThrowsException(): void {
+		$message = new Message();
+		$message->setImipMessage(true);
+		$message->setUid(1);
+		$message->setMailboxId(100);
+		$mailbox = new Mailbox();
+		$mailbox->setId(100);
+		$mailbox->setAccountId(200);
+		$mailAccount = new MailAccount();
+		$mailAccount->setId(200);
+		$mailAccount->setEmail('[email protected]');
+		$mailAccount->setUserId('vincent');
+		$account = new Account($mailAccount);
+		$imapMessage = $this->createMock(IMAPMessage::class);
+		$imapMessage->scheduling[] = ['method' => 'REQUEST', 'contents' => 'VCALENDAR'];
+		$addressList = $this->createMock(AddressList::class);
+		$address = $this->createMock(Address::class);
+
+		$this->messageMapper->expects(self::once())
+			->method('findIMipMessagesAscending')
+			->willReturn([$message]);
+		$this->mailboxMapper->expects(self::once())
+			->method('findById')
+			->willReturn($mailbox);
+		$this->accountService->expects(self::once())
+			->method('findById')
+			->willReturn($account);
+		$this->mailManager->expects(self::once())
+			->method('getImapMessagesForScheduleProcessing')
+			->with($account, $mailbox, [$message->getUid()])
+			->willReturn([$imapMessage]);
+		$imapMessage->expects(self::once())
+			->method('getUid')
+			->willReturn(1);
+		$imapMessage->expects(self::once())
+			->method('getFrom')
+			->willReturn($addressList);
+		$addressList->expects(self::once())
+			->method('first')
+			->willReturn($address);
+		$address->expects(self::once())
+			->method('getEmail')
+			->willReturn('[email protected]');
+		$this->calendarManager->expects(self::once())
+			->method('handleIMipRequest')
+			->willThrowException(new \Exception('Calendar error'));
+		$this->logger->expects(self::once())
+			->method('error')
+			->with(
+				'iMIP message processing failed',
+				self::callback(function ($context) use ($message, $mailbox) {
+					return isset($context['exception'])
+						&& $context['messageId'] === $message->getId()
+						&& $context['mailboxId'] === $mailbox->getId();
+				})
+			);
+		$this->messageMapper->expects(self::once())
+			->method('updateImipData')
+			->with(self::callback(function (Message $msg) {
+				return $msg->isImipProcessed() === true && $msg->isImipError() === true;
+			}));
+
+		$this->service->process();
+	}
 }