From 531b73f6c3e24c4c5c913181d6768e5fe1833d9a Mon Sep 17 00:00:00 2001
From: escoand <escoand@users.noreply.github.com>
Date: Wed, 25 Sep 2024 12:29:12 +0200
Subject: [PATCH] fix(dav): don't crash subscription on invalid calendar object

Signed-off-by: escoand <escoand@users.noreply.github.com>
---
 .../WebcalCaching/RefreshWebcalService.php    | 19 +++++++++++++------
 .../RefreshWebcalServiceTest.php              |  4 ++--
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php b/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php
index 102571c8ef9ef..05ee43eaffe13 100644
--- a/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php
+++ b/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php
@@ -12,6 +12,7 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use Psr\Log\LoggerInterface;
 use Sabre\DAV\Exception\BadRequest;
+use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\PropPatch;
 use Sabre\VObject\Component;
 use Sabre\VObject\DateTimeParser;
@@ -99,7 +100,13 @@ public function refreshSubscription(string $principalUri, string $uri) {
 					continue;
 				}
 
-				$denormalized = $this->calDavBackend->getDenormalizedData($vObject->serialize());
+				try {
+					$denormalized = $this->calDavBackend->getDenormalizedData($vObject->serialize());
+				} catch (InvalidDataException|Forbidden $ex) {
+					$this->logger->warning('Unable to denormalize calendar object from subscription {subscriptionId}', ['exception' => $ex, 'subscriptionId' => $subscription['id'], 'source' => $subscription['source']]);
+					continue;
+				}
+
 				// Find all identical sets and remove them from the update
 				if (isset($localData[$uid]) && $denormalized['etag'] === $localData[$uid]['etag']) {
 					unset($localData[$uid]);
@@ -124,19 +131,19 @@ public function refreshSubscription(string $principalUri, string $uri) {
 				try {
 					$objectUri = $this->getRandomCalendarObjectUri();
 					$this->calDavBackend->createCalendarObject($subscription['id'], $objectUri, $vObject->serialize(), CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION);
-				} catch (NoInstancesException | BadRequest $ex) {
-					$this->logger->error('Unable to create calendar object from subscription {subscriptionId}', ['exception' => $ex, 'subscriptionId' => $subscription['id'], 'source' => $subscription['source']]);
+				} catch (NoInstancesException|BadRequest $ex) {
+					$this->logger->warning('Unable to create calendar object from subscription {subscriptionId}', ['exception' => $ex, 'subscriptionId' => $subscription['id'], 'source' => $subscription['source']]);
 				}
 			}
 
 			$ids = array_map(static function ($dataSet): int {
-				return (int) $dataSet['id'];
+				return (int)$dataSet['id'];
 			}, $localData);
 			$uris = array_map(static function ($dataSet): string {
 				return $dataSet['uri'];
 			}, $localData);
 
-			if(!empty($ids) && !empty($uris)) {
+			if (!empty($ids) && !empty($uris)) {
 				// Clean up on aisle 5
 				// The only events left over in the $localData array should be those that don't exist upstream
 				// All deleted VObjects from upstream are removed
@@ -150,7 +157,7 @@ public function refreshSubscription(string $principalUri, string $uri) {
 
 			$this->updateSubscription($subscription, $mutations);
 		} catch (ParseException $ex) {
-			$this->logger->error("Subscription {subscriptionId} could not be refreshed due to a parsing error", ['exception' => $ex, 'subscriptionId' => $subscription['id']]);
+			$this->logger->error('Subscription {subscriptionId} could not be refreshed due to a parsing error', ['exception' => $ex, 'subscriptionId' => $subscription['id']]);
 		}
 	}
 
diff --git a/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php b/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php
index ed140fd76ff3d..0ca57333b3588 100644
--- a/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php
+++ b/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php
@@ -252,7 +252,7 @@ public function testRunCreateCalendarNoException(string $body, string $contentTy
 			->willThrowException($noInstanceException);
 
 		$this->logger->expects(self::once())
-			->method('error')
+			->method('warning')
 			->with('Unable to create calendar object from subscription {subscriptionId}', ['exception' => $noInstanceException, 'subscriptionId' => '42', 'source' => 'webcal://foo.bar/bla2']);
 
 		$refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123');
@@ -302,7 +302,7 @@ public function testRunCreateCalendarBadRequest(string $body, string $contentTyp
 			->willThrowException($badRequestException);
 
 		$this->logger->expects(self::once())
-			->method('error')
+			->method('warning')
 			->with('Unable to create calendar object from subscription {subscriptionId}', ['exception' => $badRequestException, 'subscriptionId' => '42', 'source' => 'webcal://foo.bar/bla2']);
 
 		$refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123');