From 6bbd08f44412451d17f320b34097f6763c14320d Mon Sep 17 00:00:00 2001 From: Torben Dannhauer Date: Fri, 18 Sep 2026 23:31:28 +0200 Subject: [PATCH] fix(activesync): heal ghost collections instead of FolderSync loops Collections with a synckey but no folders cache entry cannot be repaired by incremental FolderSync (Count 0). PING Status 7 then livelocks cooperating clients. Restore the folders row from foldermap, or drop the ghost collection, instead of requesting FolderSync. --- lib/Horde/ActiveSync/Collections.php | 161 ++++++++++++++-- lib/Horde/ActiveSync/Request/Ping.php | 13 +- .../ActiveSync/BacklogPingRecoveryTest.php | 4 +- .../CollectionsGhostFolderCacheTest.php | 177 ++++++++++++++++++ .../PingCollectionLockContentionTest.php | 4 +- 5 files changed, 338 insertions(+), 21 deletions(-) create mode 100644 test/unit/Horde/ActiveSync/CollectionsGhostFolderCacheTest.php diff --git a/lib/Horde/ActiveSync/Collections.php b/lib/Horde/ActiveSync/Collections.php index 1180655a..3c75f9b1 100644 --- a/lib/Horde/ActiveSync/Collections.php +++ b/lib/Horde/ActiveSync/Collections.php @@ -1301,11 +1301,11 @@ public function pollForChanges($heartbeat, $interval, array $options = []) ) ); - if (!empty($options['pingable'])) { - if ($this->collectionsNeedFolderResync()) { - return self::COLLECTION_ERR_FOLDERSYNC_REQUIRED; - } + // Repair collection/folder-cache skew on the server. Incremental + // FolderSync cannot do this (valid hierarchy synckey → Count 0). + $this->healCollectionsMissingFolderCache(); + if (!empty($options['pingable'])) { $this->restorePingableCollectionsFromCache(); // If pinging, make sure we have pingable collections. Note we can't @@ -1702,14 +1702,45 @@ public function havePingableCollections() /** * True if a collection has item sync state but no folder cache entry. * - * The client must run FolderSync to remap folder ids (common after a - * partial cache reset or hierarchy invalidation). + * Historically this asked the client for FolderSync. Incremental FolderSync + * cannot repair that cache skew (no hierarchy changes → Count 0), so this + * now self-heals via healCollectionsMissingFolderCache() and returns false. + * Missing hierarchy synckey is still handled separately via haveHierarchy(). * * @return boolean */ public function collectionsNeedFolderResync() { + $this->healCollectionsMissingFolderCache(); + + return false; + } + + /** + * Drop or rehydrate collections that have a synckey but no folders[] entry. + * + * If the uid is still in the persistent foldermap, restore the folders + * cache entry so PING/SYNC can resolve the backend id. Otherwise the + * collection is a ghost (deleted or remapped folder) and is removed. + * Requesting FolderSync is the wrong recovery: a valid hierarchy synckey + * produces Count 0 and livelocks cooperating clients (PING 7 → FolderSync). + * + * @return boolean True if the sync cache was modified. + * @author Torben Dannhauer + */ + public function healCollectionsMissingFolderCache() + { + if ($this->_cache->countCollections() == 0) { + return false; + } + + $changed = false; + $uidToBackend = array_flip($this->_cache->getFolderMap()); + foreach ($this->_cache->getCollections(false) as $id => $collection) { + if ($id === 'RI') { + continue; + } $synckey = $collection['synckey'] ?? ''; if ($synckey === '' && !empty($collection['lastsynckey'])) { $synckey = $collection['lastsynckey']; @@ -1717,19 +1748,127 @@ public function collectionsNeedFolderResync() if ($synckey === '' || $synckey === '0') { continue; } - if (!$this->_cache->getFolder($id)) { + if ($this->_cache->getFolder($id)) { + continue; + } + + $backendId = $uidToBackend[$id] ?? ''; + if ($backendId !== '' && $backendId !== false) { + $this->_restoreFolderCacheEntry($id, (string)$backendId, $collection); $this->_logger->info( sprintf( - 'COLLECTIONS: Collection %s has sync state but no folder cache entry; FolderSync required.', - $id + 'COLLECTIONS: Restored folder cache entry for %s from foldermap (%s).', + $id, + $backendId ) ); + $changed = true; + continue; + } - return true; + $this->_logger->notice( + sprintf( + 'COLLECTIONS: Dropping collection %s; has sync state but no folder cache entry or foldermap mapping.', + $id + ) + ); + $this->_cache->removeCollection($id, true); + unset($this->_collections[$id]); + $changed = true; + } + + if ($changed) { + $this->save(); + } + + return $changed; + } + + /** + * Rebuild a folders[] cache row from a known backend id. + * + * @param string $uid EAS folder uid. + * @param string $backendId Backend folder id. + * @param array $collection Collection cache row. + */ + protected function _restoreFolderCacheEntry($uid, $backendId, array $collection) + { + $class = !empty($collection['class']) + ? $collection['class'] + : $this->_collectionClassFromFolderUid($uid); + $type = !empty($collection['type']) + ? $collection['type'] + : $this->_folderTypeFromClass($class); + + $version = !empty($this->_as->device->version) + ? $this->_as->device->version + : Horde_ActiveSync::VERSION_FOURTEEN; + $folder = new Horde_ActiveSync_Message_Folder(['protocolversion' => $version]); + $folder->serverid = $uid; + $folder->_serverid = $backendId; + $folder->type = $type; + $this->_cache->updateFolder($folder); + + $cols = $this->_cache->getCollections(false); + if (!empty($cols[$uid])) { + $cols[$uid]['serverid'] = $backendId; + if (empty($cols[$uid]['class'])) { + $cols[$uid]['class'] = $class; } + $this->_cache->updateCollection($cols[$uid]); + } + if (isset($this->_collections[$uid])) { + $this->_collections[$uid]['serverid'] = $backendId; + if (empty($this->_collections[$uid]['class'])) { + $this->_collections[$uid]['class'] = $class; + } + } + } + + /** + * Derive a collection class from the deterministic EAS uid prefix. + * + * @param string $uid EAS folder uid. + * + * @return string Horde_ActiveSync::CLASS_* value. + */ + protected function _collectionClassFromFolderUid($uid) + { + switch (substr($uid, 0, 1)) { + case 'A': + return Horde_ActiveSync::CLASS_CALENDAR; + case 'C': + return Horde_ActiveSync::CLASS_CONTACTS; + case 'T': + return Horde_ActiveSync::CLASS_TASKS; + case 'N': + return Horde_ActiveSync::CLASS_NOTES; + default: + return Horde_ActiveSync::CLASS_EMAIL; } + } - return false; + /** + * Default folder type for a collection class when restoring a cache row. + * + * @param string $class Horde_ActiveSync::CLASS_* value. + * + * @return integer Horde_ActiveSync::FOLDER_TYPE_* value. + */ + protected function _folderTypeFromClass($class) + { + switch ($class) { + case Horde_ActiveSync::CLASS_CALENDAR: + return Horde_ActiveSync::FOLDER_TYPE_USER_APPOINTMENT; + case Horde_ActiveSync::CLASS_CONTACTS: + return Horde_ActiveSync::FOLDER_TYPE_USER_CONTACT; + case Horde_ActiveSync::CLASS_TASKS: + return Horde_ActiveSync::FOLDER_TYPE_USER_TASK; + case Horde_ActiveSync::CLASS_NOTES: + return Horde_ActiveSync::FOLDER_TYPE_USER_NOTE; + default: + return Horde_ActiveSync::FOLDER_TYPE_USER_MAIL; + } } /** diff --git a/lib/Horde/ActiveSync/Request/Ping.php b/lib/Horde/ActiveSync/Request/Ping.php index 403015ed..4152b810 100644 --- a/lib/Horde/ActiveSync/Request/Ping.php +++ b/lib/Horde/ActiveSync/Request/Ping.php @@ -151,12 +151,12 @@ protected function _handle() $this->_logger->info('Handling empty PING request.'); $isEmpty = true; $collections->loadCollectionsFromCache(); + $collections->healCollectionsMissingFolderCache(); $collections->restorePingableCollectionsFromCache(); if ($collections->collectionCount() == 0 || !$collections->havePingableCollections()) { - if ($collections->collectionsNeedFolderResync() - || ($this->_device->version >= Horde_ActiveSync::VERSION_TWELVEONE - && !$collections->haveHierarchy())) { + if ($this->_device->version >= Horde_ActiveSync::VERSION_TWELVEONE + && !$collections->haveHierarchy()) { $this->_logger->info( 'Empty PING with stale folder or hierarchy state; requesting FolderSync.' ); @@ -220,11 +220,11 @@ protected function _handle() 'No collections loaded from explicit PING folder list; using cached collections.' ); $collections->loadCollectionsFromCache(); + $collections->healCollectionsMissingFolderCache(); $collections->restorePingableCollectionsFromCache(); if ($collections->collectionCount() == 0) { - if ($collections->collectionsNeedFolderResync() - || ($this->_device->version >= Horde_ActiveSync::VERSION_TWELVEONE - && !$collections->haveHierarchy())) { + if ($this->_device->version >= Horde_ActiveSync::VERSION_TWELVEONE + && !$collections->haveHierarchy()) { $this->_logger->info( 'Explicit PING with stale folder or hierarchy state; requesting FolderSync.' ); @@ -246,6 +246,7 @@ protected function _handle() } else { // No FOLDERS supplied, use the cache. $collections->loadCollectionsFromCache(); + $collections->healCollectionsMissingFolderCache(); if ($collections->collectionCount() == 0) { $this->_logger->warn('Empty PING request with no cached collections. Request full PING.'); $this->_statusCode = self::STATUS_MISSING; diff --git a/test/unit/Horde/ActiveSync/BacklogPingRecoveryTest.php b/test/unit/Horde/ActiveSync/BacklogPingRecoveryTest.php index de34703f..ce5507ab 100644 --- a/test/unit/Horde/ActiveSync/BacklogPingRecoveryTest.php +++ b/test/unit/Horde/ActiveSync/BacklogPingRecoveryTest.php @@ -180,7 +180,7 @@ protected function _collectionsFixture(Horde_ActiveSync_SyncCache $cache) 'initCollectionState', 'updateCollectionsFromCache', 'checkStaleRequest', - 'collectionsNeedFolderResync', + 'healCollectionsMissingFolderCache', 'restorePingableCollectionsFromCache', 'havePingableCollections', 'haveHierarchy', @@ -191,7 +191,7 @@ protected function _collectionsFixture(Horde_ActiveSync_SyncCache $cache) ]) ->getMock(); - $collections->method('collectionsNeedFolderResync')->willReturn(false); + $collections->method('healCollectionsMissingFolderCache')->willReturn(false); $collections->method('havePingableCollections')->willReturn(true); $collections->method('haveHierarchy')->willReturn(true); $collections->method('checkStaleRequest')->willReturn(false); diff --git a/test/unit/Horde/ActiveSync/CollectionsGhostFolderCacheTest.php b/test/unit/Horde/ActiveSync/CollectionsGhostFolderCacheTest.php new file mode 100644 index 00000000..22c6e584 --- /dev/null +++ b/test/unit/Horde/ActiveSync/CollectionsGhostFolderCacheTest.php @@ -0,0 +1,177 @@ + + * @category Horde + * @copyright 2026 The Horde Project + * @license http://www.horde.org/licenses/gpl GPLv2 + * @package ActiveSync + */ + +namespace Horde\ActiveSync; + +use Horde_ActiveSync; +use Horde_ActiveSync_Collections; +use Horde_ActiveSync_Log_Logger; +use Horde_ActiveSync_State_Base; +use Horde_ActiveSync_SyncCache; +use Horde_Log_Handler_Null; +use PHPUnit\Framework\Attributes\CoversNothing; +use PHPUnit\Framework\TestCase; + +#[CoversNothing] +class CollectionsGhostFolderCacheTest extends TestCase +{ + private const GHOST_UID = 'A99cfc101'; + private const LIVE_UID = 'Ad225f47d'; + private const LIVE_BACKEND = 'Calendar:livecalendarid'; + private const MAP_UID = 'A11111111'; + private const MAP_BACKEND = 'Calendar:stillhere'; + + protected function _newCollections(array $stored): Horde_ActiveSync_Collections + { + $stored = array_merge([ + 'confirmed_synckeys' => [], + 'lasthbsyncstarted' => false, + 'lastsyncendnormal' => false, + 'timestamp' => time(), + 'wait' => false, + 'hbinterval' => false, + 'folders' => [], + 'foldermap' => [], + 'hierarchy' => '{00000000-0000-0000-0000-000000000001}3', + 'collections' => [], + 'pingheartbeat' => false, + ], $stored); + + $state = $this->createMock(Horde_ActiveSync_State_Base::class); + $state->method('getSyncCache')->willReturn($stored); + $state->method('saveSyncCache'); + + $cache = new Horde_ActiveSync_SyncCache( + $state, + 'device', + 'user', + new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null()) + ); + + $as = $this->createMock(Horde_ActiveSync::class); + $as->logger = new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null()); + $as->device = (object) [ + 'id' => 'device', + 'version' => Horde_ActiveSync::VERSION_FOURTEEN, + ]; + + return new Horde_ActiveSync_Collections($cache, $as); + } + + protected function _cache(Horde_ActiveSync_Collections $collections): Horde_ActiveSync_SyncCache + { + $prop = new \ReflectionProperty(Horde_ActiveSync_Collections::class, '_cache'); + $prop->setAccessible(true); + + return $prop->getValue($collections); + } + + public function testHealDropsGhostCollectionWithoutFolderMapEntry() + { + $collections = $this->_newCollections([ + 'folders' => [ + self::LIVE_UID => [ + 'class' => Horde_ActiveSync::CLASS_CALENDAR, + 'serverid' => self::LIVE_BACKEND, + 'type' => Horde_ActiveSync::FOLDER_TYPE_USER_APPOINTMENT, + ], + ], + 'foldermap' => [ + self::LIVE_BACKEND => self::LIVE_UID, + ], + 'collections' => [ + self::LIVE_UID => [ + 'class' => Horde_ActiveSync::CLASS_CALENDAR, + 'synckey' => '{00000000-0000-0000-0000-000000000002}80', + 'pingable' => true, + 'serverid' => self::LIVE_BACKEND, + ], + self::GHOST_UID => [ + 'class' => Horde_ActiveSync::CLASS_CALENDAR, + 'synckey' => '{00000000-0000-0000-0000-000000000003}4', + 'pingable' => true, + ], + ], + ]); + + $this->assertTrue($collections->healCollectionsMissingFolderCache()); + + $cache = $this->_cache($collections); + $this->assertFalse($cache->collectionExists(self::GHOST_UID)); + $this->assertNotFalse($cache->getFolder(self::LIVE_UID)); + $this->assertTrue($cache->collectionExists(self::LIVE_UID)); + $this->assertFalse($collections->collectionsNeedFolderResync()); + } + + public function testHealRestoresFolderCacheFromFolderMap() + { + $collections = $this->_newCollections([ + 'folders' => [], + 'foldermap' => [ + self::MAP_BACKEND => self::MAP_UID, + ], + 'collections' => [ + self::MAP_UID => [ + 'class' => Horde_ActiveSync::CLASS_CALENDAR, + 'synckey' => '{00000000-0000-0000-0000-000000000004}2', + 'pingable' => true, + ], + ], + ]); + + $this->assertTrue($collections->healCollectionsMissingFolderCache()); + + $cache = $this->_cache($collections); + $folder = $cache->getFolder(self::MAP_UID); + $this->assertNotFalse($folder); + $this->assertSame(self::MAP_BACKEND, $folder['serverid']); + $this->assertSame(Horde_ActiveSync::CLASS_CALENDAR, $folder['class']); + $this->assertTrue($cache->collectionExists(self::MAP_UID)); + $this->assertFalse($collections->collectionsNeedFolderResync()); + } + + public function testHealLeavesHealthyCollectionsUntouched() + { + $collections = $this->_newCollections([ + 'folders' => [ + self::LIVE_UID => [ + 'class' => Horde_ActiveSync::CLASS_CALENDAR, + 'serverid' => self::LIVE_BACKEND, + 'type' => Horde_ActiveSync::FOLDER_TYPE_USER_APPOINTMENT, + ], + ], + 'foldermap' => [ + self::LIVE_BACKEND => self::LIVE_UID, + ], + 'collections' => [ + self::LIVE_UID => [ + 'class' => Horde_ActiveSync::CLASS_CALENDAR, + 'synckey' => '{00000000-0000-0000-0000-000000000002}80', + 'pingable' => true, + 'serverid' => self::LIVE_BACKEND, + ], + ], + ]); + + $this->assertFalse($collections->healCollectionsMissingFolderCache()); + $this->assertFalse($collections->collectionsNeedFolderResync()); + $this->assertTrue($this->_cache($collections)->collectionExists(self::LIVE_UID)); + } +} diff --git a/test/unit/Horde/ActiveSync/PingCollectionLockContentionTest.php b/test/unit/Horde/ActiveSync/PingCollectionLockContentionTest.php index 6448cc78..0e1a16f1 100644 --- a/test/unit/Horde/ActiveSync/PingCollectionLockContentionTest.php +++ b/test/unit/Horde/ActiveSync/PingCollectionLockContentionTest.php @@ -56,7 +56,7 @@ public function testPollForChangesSkipsCollectionOnLockContention() 'initCollectionState', 'updateCollectionsFromCache', 'checkStaleRequest', - 'collectionsNeedFolderResync', + 'healCollectionsMissingFolderCache', 'restorePingableCollectionsFromCache', 'havePingableCollections', 'haveHierarchy', @@ -66,7 +66,7 @@ public function testPollForChangesSkipsCollectionOnLockContention() ]) ->getMock(); - $collections->method('collectionsNeedFolderResync')->willReturn(false); + $collections->method('healCollectionsMissingFolderCache')->willReturn(false); $collections->method('havePingableCollections')->willReturn(true); $collections->method('haveHierarchy')->willReturn(true); $collections->method('checkStaleRequest')->willReturn(false);