Merge pull request #6901 from nextcloud/bugfix/4014/catch_parsing_error

catch errors when parsing calendar data for calendar query requests

6 files changed, 44 insertions, 5 deletions

diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php
index e96c3f28064..d9851bf92c8 100644
--- a/apps/dav/appinfo/v1/caldav.php
+++ b/apps/dav/appinfo/v1/caldav.php
@@ -49,8 +49,9 @@ $principalBackend = new Principal(
 $db = \OC::$server->getDatabaseConnection();
 $userManager = \OC::$server->getUserManager();
 $random = \OC::$server->getSecureRandom();
+$logger = \OC::$server->getLogger();
 $dispatcher = \OC::$server->getEventDispatcher();
-$calDavBackend = new CalDavBackend($db, $principalBackend, $userManager, \OC::$server->getGroupManager(), $random, $dispatcher, true);
+$calDavBackend = new CalDavBackend($db, $principalBackend, $userManager, \OC::$server->getGroupManager(), $random, $logger, $dispatcher, true);
 
 $debugging = \OC::$server->getConfig()->getSystemValue('debug', false);
 $sendInvitations = \OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes';
diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php
index 9045a62cde4..2f591a262a2 100644
--- a/apps/dav/lib/CalDAV/CalDavBackend.php
+++ b/apps/dav/lib/CalDAV/CalDavBackend.php
@@ -37,6 +37,7 @@ use OCA\DAV\Connector\Sabre\Principal;
 use OCA\DAV\DAV\Sharing\Backend;
 use OCP\IDBConnection;
 use OCP\IGroupManager;
+use OCP\ILogger;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
@@ -56,6 +57,8 @@ use Sabre\VObject\Component\VCalendar;
 use Sabre\VObject\Component\VEvent;
 use Sabre\VObject\Component\VTimeZone;
 use Sabre\VObject\DateTimeParser;
+use Sabre\VObject\InvalidDataException;
+use Sabre\VObject\ParseException;
 use Sabre\VObject\Property;
 use Sabre\VObject\Reader;
 use Sabre\VObject\Recur\EventIterator;
@@ -152,6 +155,9 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 	/** @var ISecureRandom */
 	private $random;
 
+	/** @var ILogger */
+	private $logger;
+
 	/** @var EventDispatcherInterface */
 	private $dispatcher;
 
@@ -169,6 +175,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 	 * @param IUserManager $userManager
 	 * @param IGroupManager $groupManager
 	 * @param ISecureRandom $random
+	 * @param ILogger $logger
 	 * @param EventDispatcherInterface $dispatcher
 	 * @param bool $legacyEndpoint
 	 */
@@ -177,6 +184,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 								IUserManager $userManager,
 								IGroupManager $groupManager,
 								ISecureRandom $random,
+								ILogger $logger,
 								EventDispatcherInterface $dispatcher,
 								$legacyEndpoint = false) {
 		$this->db = $db;
@@ -184,6 +192,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 		$this->userManager = $userManager;
 		$this->sharingBackend = new Backend($this->db, $this->userManager, $groupManager, $principalBackend, 'calendar');
 		$this->random = $random;
+		$this->logger = $logger;
 		$this->dispatcher = $dispatcher;
 		$this->legacyEndpoint = $legacyEndpoint;
 	}
@@ -1219,7 +1228,25 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 		$result = [];
 		while($row = $stmt->fetch(\PDO::FETCH_ASSOC)) {
 			if ($requirePostFilter) {
-				if (!$this->validateFilterForObject($row, $filters)) {
+				// validateFilterForObject will parse the calendar data
+				// catch parsing errors
+				try {
+					$matches = $this->validateFilterForObject($row, $filters);
+				} catch(ParseException $ex) {
+					$this->logger->logException($ex, [
+						'app' => 'dav',
+						'message' => 'Caught parsing exception for calendar data. This usually indicates invalid calendar data. calendar-id:'.$calendarId.' uri:'.$row['uri']
+					]);
+					continue;
+				} catch (InvalidDataException $ex) {
+					$this->logger->logException($ex, [
+						'app' => 'dav',
+						'message' => 'Caught invalid data exception for calendar data. This usually indicates invalid calendar data. calendar-id:'.$calendarId.' uri:'.$row['uri']
+					]);
+					continue;
+				}
+
+				if (!$matches) {
 					continue;
 				}
 			}
diff --git a/apps/dav/lib/Command/CreateCalendar.php b/apps/dav/lib/Command/CreateCalendar.php
index 1ef859e0631..190e4aa339f 100644
--- a/apps/dav/lib/Command/CreateCalendar.php
+++ b/apps/dav/lib/Command/CreateCalendar.php
@@ -78,10 +78,11 @@ class CreateCalendar extends Command {
 			$this->groupManager
 		);
 		$random = \OC::$server->getSecureRandom();
+		$logger = \OC::$server->getLogger();
 		$dispatcher = \OC::$server->getEventDispatcher();
 
 		$name = $input->getArgument('name');
-		$caldav = new CalDavBackend($this->dbConnection, $principalBackend, $this->userManager, $this->groupManager, $random, $dispatcher);
+		$caldav = new CalDavBackend($this->dbConnection, $principalBackend, $this->userManager, $this->groupManager, $random, $logger, $dispatcher);
 		$caldav->createCalendar("principals/users/$user", $name, []);
 	}
 }
diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php
index e4ba1f2c02a..7af1745cd74 100644
--- a/apps/dav/lib/RootCollection.php
+++ b/apps/dav/lib/RootCollection.php
@@ -40,6 +40,7 @@ class RootCollection extends SimpleCollection {
 	public function __construct() {
 		$config = \OC::$server->getConfig();
 		$random = \OC::$server->getSecureRandom();
+		$logger = \OC::$server->getLogger();
 		$userManager = \OC::$server->getUserManager();
 		$groupManager = \OC::$server->getGroupManager();
 		$db = \OC::$server->getDatabaseConnection();
@@ -61,7 +62,7 @@ class RootCollection extends SimpleCollection {
 		$systemPrincipals->disableListing = $disableListing;
 		$filesCollection = new Files\RootCollection($userPrincipalBackend, 'principals/users');
 		$filesCollection->disableListing = $disableListing;
-		$caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $random, $dispatcher);
+		$caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $random, $logger, $dispatcher);
 		$calendarRoot = new CalendarRoot($userPrincipalBackend, $caldavBackend, 'principals/users');
 		$calendarRoot->disableListing = $disableListing;
 		$publicCalendarRoot = new PublicCalendarRoot($caldavBackend);
diff --git a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php
index 90bf860b24d..2a01bd425c0 100644
--- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php
+++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php
@@ -28,6 +28,7 @@ namespace OCA\DAV\Tests\unit\CalDAV;
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\Connector\Sabre\Principal;
 use OCP\IGroupManager;
+use OCP\ILogger;
 use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
 use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet;
@@ -57,6 +58,8 @@ abstract class AbstractCalDavBackend extends TestCase {
 
 	/** @var ISecureRandom */
 	private $random;
+	/** @var ILogger */
+	private $logger;
 
 	const UNIT_TEST_USER = 'principals/users/caldav-unit-test';
 	const UNIT_TEST_USER1 = 'principals/users/caldav-unit-test1';
@@ -84,7 +87,8 @@ abstract class AbstractCalDavBackend extends TestCase {
 
 		$db = \OC::$server->getDatabaseConnection();
 		$this->random = \OC::$server->getSecureRandom();
-		$this->backend = new CalDavBackend($db, $this->principal, $this->userManager, $this->groupManager, $this->random, $this->dispatcher);
+		$this->logger = $this->createMock(ILogger::class);
+		$this->backend = new CalDavBackend($db, $this->principal, $this->userManager, $this->groupManager, $this->random, $this->logger, $this->dispatcher);
 
 		$this->cleanUpBackend();
 	}
diff --git a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php
index 82f4161c206..57707c6c0a4 100644
--- a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php
+++ b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php
@@ -34,6 +34,7 @@ use OCP\IGroupManager;
 use OCP\IL10N;
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\CalDAV\PublicCalendarRoot;
+use OCP\ILogger;
 use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -64,6 +65,8 @@ class PublicCalendarRootTest extends TestCase {
 
 	/** @var ISecureRandom */
 	private $random;
+	/** @var ILogger */
+	private $logger;
 
 	public function setUp() {
 		parent::setUp();
@@ -73,6 +76,7 @@ class PublicCalendarRootTest extends TestCase {
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->random = \OC::$server->getSecureRandom();
+		$this->logger = $this->createMock(ILogger::class);
 		$dispatcher = $this->createMock(EventDispatcherInterface::class);
 
 		$this->principal->expects($this->any())->method('getGroupMembership')
@@ -85,6 +89,7 @@ class PublicCalendarRootTest extends TestCase {
 			$this->userManager,
 			$this->groupManager,
 			$this->random,
+			$this->logger,
 			$dispatcher
 		);