From 5060c3225df4bc15db34fe7748d9742c46818d90 Mon Sep 17 00:00:00 2001 From: olen Date: Mon, 9 Feb 2026 21:02:37 +0100 Subject: [PATCH] fix(caldav): compute etag without dtstamp for subscriptions Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Olen Signed-off-by: SebastianKrupinski Signed-off-by: Daniel Kesselberg --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/lib/CalDAV/CalDavBackend.php | 18 +- .../lib/CalDAV/CalendarObjectEtagHelper.php | 26 +++ .../WebcalCaching/RefreshWebcalService.php | 4 +- .../tests/unit/CalDAV/CalDavBackendTest.php | 35 +++ .../CalDAV/CalendarObjectEtagHelperTest.php | 53 +++++ .../RefreshWebcalServiceTest.php | 199 +++++++++++++++++- 8 files changed, 328 insertions(+), 9 deletions(-) create mode 100644 apps/dav/lib/CalDAV/CalendarObjectEtagHelper.php create mode 100644 apps/dav/tests/unit/CalDAV/CalendarObjectEtagHelperTest.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 310f1ec83be3d..890b505c54ecf 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -59,6 +59,7 @@ 'OCA\\DAV\\CalDAV\\CalendarImpl' => $baseDir . '/../lib/CalDAV/CalendarImpl.php', 'OCA\\DAV\\CalDAV\\CalendarManager' => $baseDir . '/../lib/CalDAV/CalendarManager.php', 'OCA\\DAV\\CalDAV\\CalendarObject' => $baseDir . '/../lib/CalDAV/CalendarObject.php', + 'OCA\\DAV\\CalDAV\\CalendarObjectEtagHelper' => $baseDir . '/../lib/CalDAV/CalendarObjectEtagHelper.php', 'OCA\\DAV\\CalDAV\\CalendarProvider' => $baseDir . '/../lib/CalDAV/CalendarProvider.php', 'OCA\\DAV\\CalDAV\\CalendarRoot' => $baseDir . '/../lib/CalDAV/CalendarRoot.php', 'OCA\\DAV\\CalDAV\\DefaultCalendarValidator' => $baseDir . '/../lib/CalDAV/DefaultCalendarValidator.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 98a49d46284dd..10cc6591bad22 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -74,6 +74,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\CalDAV\\CalendarImpl' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarImpl.php', 'OCA\\DAV\\CalDAV\\CalendarManager' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarManager.php', 'OCA\\DAV\\CalDAV\\CalendarObject' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarObject.php', + 'OCA\\DAV\\CalDAV\\CalendarObjectEtagHelper' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarObjectEtagHelper.php', 'OCA\\DAV\\CalDAV\\CalendarProvider' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarProvider.php', 'OCA\\DAV\\CalDAV\\CalendarRoot' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarRoot.php', 'OCA\\DAV\\CalDAV\\DefaultCalendarValidator' => __DIR__ . '/..' . '/../lib/CalDAV/DefaultCalendarValidator.php', diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index d181d25f7644c..bd3701fb14ca3 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -1537,7 +1537,7 @@ public function findCalendarObjectByUid(int $calendarId, string $uid, int $calen #[\Override] public function createCalendarObject($calendarId, $objectUri, $calendarData, $calendarType = self::CALENDAR_TYPE_CALENDAR) { $this->cachedObjects = []; - $extraData = $this->getDenormalizedData($calendarData); + $extraData = $this->getDenormalizedData($calendarData, $calendarType); return $this->atomic(function () use ($calendarId, $objectUri, $calendarData, $extraData, $calendarType) { // Try to detect duplicate uids in the target collection @@ -1615,7 +1615,7 @@ public function createCalendarObject($calendarId, $objectUri, $calendarData, $ca #[\Override] public function updateCalendarObject($calendarId, $objectUri, $calendarData, $calendarType = self::CALENDAR_TYPE_CALENDAR) { $this->cachedObjects = []; - $extraData = $this->getDenormalizedData($calendarData); + $extraData = $this->getDenormalizedData($calendarData, $calendarType); return $this->atomic(function () use ($calendarId, $objectUri, $calendarData, $extraData, $calendarType) { // Read the object before overwriting it so the update event can carry @@ -3401,18 +3401,22 @@ public function restoreChanges(int $calendarId, int $calendarType = self::CALEND * * uid - value of the UID property * * @param string $calendarData + * @param int $calendarType * @return array */ - public function getDenormalizedData(string $calendarData): array { + public function getDenormalizedData(string $calendarData, int $calendarType = self::CALENDAR_TYPE_CALENDAR): array { - $derived = [ - 'etag' => md5($calendarData), - 'size' => strlen($calendarData), - ]; // validate data and extract base component /** @var VCalendar $vObject */ $vObject = Reader::read($calendarData); + $derived = [ + 'etag' => $calendarType === self::CALENDAR_TYPE_SUBSCRIPTION + ? CalendarObjectEtagHelper::computeWithoutDtstamp($vObject) + : md5($calendarData), + 'size' => strlen($calendarData), + ]; + // Extracts componentType, uid, classification, firstOccurence and lastOccurence from a single event/todo/journal component. // RECURRENCE-ID is irrelevant here: it plays no part in this computation, so it works just as well on a recurrence exception as // it does on a series master or a non-recurring component. diff --git a/apps/dav/lib/CalDAV/CalendarObjectEtagHelper.php b/apps/dav/lib/CalDAV/CalendarObjectEtagHelper.php new file mode 100644 index 0000000000000..5debbe2f1137a --- /dev/null +++ b/apps/dav/lib/CalDAV/CalendarObjectEtagHelper.php @@ -0,0 +1,26 @@ +getComponents() as $component) { + unset($component->DTSTAMP); + } + return md5($vObject->serialize()); + } +} diff --git a/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php b/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php index 0ed3a21957934..f6101bdd5432a 100644 --- a/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php +++ b/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php @@ -10,6 +10,7 @@ namespace OCA\DAV\CalDAV\WebcalCaching; use OCA\DAV\CalDAV\CalDavBackend; +use OCA\DAV\CalDAV\CalendarObjectEtagHelper; use OCA\DAV\CalDAV\Import\ImportService; use OCP\AppFramework\Utility\ITimeFactory; use Psr\Log\LoggerInterface; @@ -113,7 +114,8 @@ public function refreshSubscription(string $principalUri, string $uri) { $sObject = $vObject->serialize(); $uid = $vBase->UID->getValue(); - $etag = md5($sObject); + + $etag = CalendarObjectEtagHelper::computeWithoutDtstamp($vObject); // No existing object with this UID, create it if (!isset($existingObjects[$uid])) { diff --git a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php index 3abc8084b4f88..0e503fe91e8be 100644 --- a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php +++ b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php @@ -15,6 +15,7 @@ use DateTimeZone; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\Calendar; +use OCA\DAV\CalDAV\CalendarObjectEtagHelper; use OCA\DAV\CalDAV\Federation\FederatedCalendarEntity; use OCA\DAV\DAV\Sharing\Plugin as SharingPlugin; use OCA\DAV\Events\CalendarDeletedEvent; @@ -27,6 +28,7 @@ use Sabre\DAV\PropPatch; use Sabre\DAV\Xml\Property\Href; use Sabre\DAVACL\IACL; +use Sabre\VObject\Reader; use function time; /** @@ -1184,6 +1186,39 @@ public function testSameUriSameIdForDifferentCalendarTypes(): void { $this->assertEquals($calData2, $this->backend->getCalendarObject($subscriptionId, $uri, CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION)['calendardata']); } + public function testEtagIgnoresDtstampOnlyForSubscriptions(): void { + $calendarId = $this->createTestCalendar(); + $subscriptionId = $this->createTestSubscription(); + + $uri = static::getUniqueID('calobj'); + $calData = <<backend->createCalendarObject($calendarId, $uri, $calData); + $this->backend->createCalendarObject($subscriptionId, $uri, $calData, CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION); + + $this->assertEquals('"' . md5($calData) . '"', $this->backend->getCalendarObject($calendarId, $uri)['etag']); + $this->assertEquals('"' . CalendarObjectEtagHelper::computeWithoutDtstamp(Reader::read($calData)) . '"', $this->backend->getCalendarObject($subscriptionId, $uri, CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION)['etag']); + + $calendarEtag = $this->backend->updateCalendarObject($calendarId, $uri, $calDataNewDtstamp); + $subscriptionEtag = $this->backend->updateCalendarObject($subscriptionId, $uri, $calDataNewDtstamp, CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION); + + $this->assertEquals('"' . md5($calDataNewDtstamp) . '"', $calendarEtag); + $this->assertEquals('"' . CalendarObjectEtagHelper::computeWithoutDtstamp(Reader::read($calData)) . '"', $subscriptionEtag); + } + public function testPurgeAllCachedEventsForSubscription(): void { $subscriptionId = $this->createTestSubscription(); $uri = static::getUniqueID('calobj'); diff --git a/apps/dav/tests/unit/CalDAV/CalendarObjectEtagHelperTest.php b/apps/dav/tests/unit/CalDAV/CalendarObjectEtagHelperTest.php new file mode 100644 index 0000000000000..dec091aaace59 --- /dev/null +++ b/apps/dav/tests/unit/CalDAV/CalendarObjectEtagHelperTest.php @@ -0,0 +1,53 @@ +assertSame( + CalendarObjectEtagHelper::computeWithoutDtstamp(Reader::read(self::CALENDAR_DATA)), + CalendarObjectEtagHelper::computeWithoutDtstamp(Reader::read($changed)), + ); + } + + public function testDetectsContentChange(): void { + $changed = str_replace('SUMMARY:Test Event', 'SUMMARY:Renamed Event', self::CALENDAR_DATA); + + $this->assertNotSame( + CalendarObjectEtagHelper::computeWithoutDtstamp(Reader::read(self::CALENDAR_DATA)), + CalendarObjectEtagHelper::computeWithoutDtstamp(Reader::read($changed)), + ); + } + + public function testMatchesAfterSerializeRoundTrip(): void { + $vObject = Reader::read("BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Test//Test//EN\r\nBEGIN:VEVENT\r\nUID:etag-test\r\nDTSTAMP;X-VOBJ-ORIGINAL-TZID=America/Argentina/Buenos_Aires:20260209T120000Z\r\nDTSTART:20260301T100000Z\r\nSUMMARY:A summary that is long enough to be folded when the calendar object gets serialized\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n"); + + $this->assertSame( + CalendarObjectEtagHelper::computeWithoutDtstamp($vObject), + CalendarObjectEtagHelper::computeWithoutDtstamp(Reader::read($vObject->serialize())), + ); + } + + public function testDoesNotModifyInput(): void { + $vObject = Reader::read(self::CALENDAR_DATA); + + CalendarObjectEtagHelper::computeWithoutDtstamp($vObject); + + $this->assertSame('20260101T080000Z', $vObject->VEVENT->DTSTAMP->getValue()); + } +} diff --git a/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php b/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php index b4792ca25678d..b75e2a9e1bd2b 100644 --- a/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php +++ b/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php @@ -9,6 +9,7 @@ namespace OCA\DAV\Tests\unit\CalDAV\WebcalCaching; use OCA\DAV\CalDAV\CalDavBackend; +use OCA\DAV\CalDAV\CalendarObjectEtagHelper; use OCA\DAV\CalDAV\Import\ImportService; use OCA\DAV\CalDAV\WebcalCaching\Connection; use OCA\DAV\CalDAV\WebcalCaching\RefreshWebcalService; @@ -460,9 +461,205 @@ public function testRunCreateCalendarBadRequest(string $body, string $format, st $refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123'); } + public function testDtstampChangeDoesNotTriggerUpdate(): void { + $refreshWebcalService = new RefreshWebcalService( + $this->caldavBackend, + $this->logger, + $this->connection, + $this->timeFactory, + $this->importService + ); + + $this->caldavBackend->expects(self::once()) + ->method('getSubscriptionsForUser') + ->with('principals/users/testuser') + ->willReturn([ + [ + 'id' => '42', + 'uri' => 'sub123', + RefreshWebcalService::STRIP_TODOS => '1', + RefreshWebcalService::STRIP_ALARMS => '1', + RefreshWebcalService::STRIP_ATTACHMENTS => '1', + 'source' => 'webcal://foo.bar/bla2', + 'lastmodified' => 0, + ], + ]); + + // Feed body has a new DTSTAMP (as happens on every fetch from Google/Outlook) + $body = "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Test//Test//EN\r\nBEGIN:VEVENT\r\nUID:dtstamp-test\r\nDTSTAMP:20260209T120000Z\r\nDTSTART:20260301T100000Z\r\nSUMMARY:Test Event\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n"; + $stream = $this->createStreamFromString($body); + + $this->connection->expects(self::once()) + ->method('queryWebcalFeed') + ->willReturn(['data' => $stream, 'format' => 'ical']); + + // The stored etag was computed from the previous fetch, which had an older DTSTAMP + $existingEtag = CalendarObjectEtagHelper::computeWithoutDtstamp(VObject\Reader::read(str_replace('DTSTAMP:20260209T120000Z', 'DTSTAMP:20260101T080000Z', $body))); + + $this->caldavBackend->expects(self::once()) + ->method('getLimitedCalendarObjects') + ->willReturn([ + 'dtstamp-test' => [ + 'id' => 1, + 'uid' => 'dtstamp-test', + 'etag' => $existingEtag, + 'uri' => 'dtstamp-test.ics', + ], + ]); + + $vCalendar = VObject\Reader::read($body); + $generator = function () use ($vCalendar) { + yield $vCalendar; + }; + + $this->importService->expects(self::once()) + ->method('importText') + ->willReturn($generator()); + + // DTSTAMP-only change must NOT trigger an update + $this->caldavBackend->expects(self::never()) + ->method('updateCalendarObject'); + + $this->caldavBackend->expects(self::never()) + ->method('createCalendarObject'); + + $refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123'); + } + + public function testFoldedDtstampChangeDoesNotTriggerUpdate(): void { + $refreshWebcalService = new RefreshWebcalService( + $this->caldavBackend, + $this->logger, + $this->connection, + $this->timeFactory, + $this->importService + ); + + $this->caldavBackend->expects(self::once()) + ->method('getSubscriptionsForUser') + ->with('principals/users/testuser') + ->willReturn([ + [ + 'id' => '42', + 'uri' => 'sub123', + RefreshWebcalService::STRIP_TODOS => '1', + RefreshWebcalService::STRIP_ALARMS => '1', + RefreshWebcalService::STRIP_ATTACHMENTS => '1', + 'source' => 'webcal://foo.bar/bla2', + 'lastmodified' => 0, + ], + ]); + + // DTSTAMP with TZID parameter exceeds 75 bytes, triggering RFC 5545 content line folding + $body = "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Test//Test//EN\r\nBEGIN:VEVENT\r\nUID:folded-dtstamp-test\r\nDTSTAMP;X-VOBJ-ORIGINAL-TZID=America/Argentina/Buenos_Aires:20260209T120000Z\r\nDTSTART:20260301T100000Z\r\nSUMMARY:Test Event\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n"; + $stream = $this->createStreamFromString($body); + + $this->connection->expects(self::once()) + ->method('queryWebcalFeed') + ->willReturn(['data' => $stream, 'format' => 'ical']); + + // The stored etag was computed from the previous fetch, which had an older unfolded DTSTAMP + $existingEtag = CalendarObjectEtagHelper::computeWithoutDtstamp(VObject\Reader::read(str_replace('DTSTAMP;X-VOBJ-ORIGINAL-TZID=America/Argentina/Buenos_Aires:20260209T120000Z', 'DTSTAMP:20260101T080000Z', $body))); + + $this->caldavBackend->expects(self::once()) + ->method('getLimitedCalendarObjects') + ->willReturn([ + 'folded-dtstamp-test' => [ + 'id' => 1, + 'uid' => 'folded-dtstamp-test', + 'etag' => $existingEtag, + 'uri' => 'folded-dtstamp-test.ics', + ], + ]); + + $vCalendar = VObject\Reader::read($body); + $generator = function () use ($vCalendar) { + yield $vCalendar; + }; + + $this->importService->expects(self::once()) + ->method('importText') + ->willReturn($generator()); + + // Folded DTSTAMP change must NOT trigger an update + $this->caldavBackend->expects(self::never()) + ->method('updateCalendarObject'); + + $this->caldavBackend->expects(self::never()) + ->method('createCalendarObject'); + + $refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123'); + } + + public function testSequenceChangeTriggersUpdate(): void { + $refreshWebcalService = new RefreshWebcalService( + $this->caldavBackend, + $this->logger, + $this->connection, + $this->timeFactory, + $this->importService + ); + + $this->caldavBackend->expects(self::once()) + ->method('getSubscriptionsForUser') + ->with('principals/users/testuser') + ->willReturn([ + [ + 'id' => '42', + 'uri' => 'sub123', + RefreshWebcalService::STRIP_TODOS => '1', + RefreshWebcalService::STRIP_ALARMS => '1', + RefreshWebcalService::STRIP_ATTACHMENTS => '1', + 'source' => 'webcal://foo.bar/bla2', + 'lastmodified' => 0, + ], + ]); + + // Feed body has a new SEQUENCE, an actual content change + $body = "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Test//Test//EN\r\nBEGIN:VEVENT\r\nUID:sequence-test\r\nSEQUENCE:2\r\nDTSTAMP:20260209T120000Z\r\nDTSTART:20260301T100000Z\r\nSUMMARY:Test Event\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n"; + $stream = $this->createStreamFromString($body); + + $this->connection->expects(self::once()) + ->method('queryWebcalFeed') + ->willReturn(['data' => $stream, 'format' => 'ical']); + + // The stored etag reflects the previous SEQUENCE value + $existingEtag = CalendarObjectEtagHelper::computeWithoutDtstamp(VObject\Reader::read(str_replace('SEQUENCE:2', 'SEQUENCE:1', $body))); + + $this->caldavBackend->expects(self::once()) + ->method('getLimitedCalendarObjects') + ->willReturn([ + 'sequence-test' => [ + 'id' => 1, + 'uid' => 'sequence-test', + 'etag' => $existingEtag, + 'uri' => 'sequence-test.ics', + ], + ]); + + $vCalendar = VObject\Reader::read($body); + $generator = function () use ($vCalendar) { + yield $vCalendar; + }; + + $this->importService->expects(self::once()) + ->method('importText') + ->willReturn($generator()); + + // SEQUENCE change must trigger an update + $this->caldavBackend->expects(self::once()) + ->method('updateCalendarObject') + ->with(42, 'sequence-test.ics', $vCalendar->serialize(), CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION); + + $this->caldavBackend->expects(self::never()) + ->method('createCalendarObject'); + + $refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123'); + } + public static function identicalDataProvider(): array { $icalBody = "BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Sabre//Sabre VObject " . VObject\Version::VERSION . "//EN\r\nCALSCALE:GREGORIAN\r\nBEGIN:VEVENT\r\nUID:12345\r\nDTSTAMP:20160218T133704Z\r\nDTSTART;VALUE=DATE:19000101\r\nDTEND;VALUE=DATE:19000102\r\nRRULE:FREQ=YEARLY\r\nSUMMARY:12345's Birthday (1900)\r\nTRANSP:TRANSPARENT\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n"; - $etag = md5($icalBody); + $etag = CalendarObjectEtagHelper::computeWithoutDtstamp(VObject\Reader::read($icalBody)); return [ [