From e7194888c117c1afa0f000fabc3dc87437504032 Mon Sep 17 00:00:00 2001 From: Torben Dannhauer Date: Thu, 9 Jul 2026 23:12:34 +0200 Subject: [PATCH 1/4] fix(activesync): stabilize folder UIDs after hierarchy changes Derive new EAS folder UIDs deterministically from backend ids so concurrent FolderSync resets produce consistent mappings when contact address books change. Handle stale folder references in MoveItems with INVALID_SRC instead of HTTP 500. --- lib/Horde/ActiveSync/Driver/Base.php | 15 +- lib/Horde/ActiveSync/Request/MoveItems.php | 5 +- .../ActiveSync/FolderUidDeterministicTest.php | 99 ++++++++ .../ActiveSync/MoveItemsFolderGoneTest.php | 224 ++++++++++++++++++ 4 files changed, 340 insertions(+), 3 deletions(-) create mode 100644 test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php create mode 100644 test/unit/Horde/ActiveSync/MoveItemsFolderGoneTest.php diff --git a/lib/Horde/ActiveSync/Driver/Base.php b/lib/Horde/ActiveSync/Driver/Base.php index c4e5b8d9..936d2737 100644 --- a/lib/Horde/ActiveSync/Driver/Base.php +++ b/lib/Horde/ActiveSync/Driver/Base.php @@ -414,8 +414,19 @@ protected function _getFolderUidForBackendId($id, $type = null, $old_id = null) $rMap = array_flip($this->_typeMap); $prefix = $rMap[$type]; - // None found, generate a new UID. - $this->_tempMap[$id] = sprintf('%s%04x%04x', $prefix, mt_rand(0, 0xffff), mt_rand(0, 0xffff)); + // None found, generate a new UID. This is derived deterministically + // from the backend id (rather than mt_rand()) so that concurrent or + // repeated cache rebuilds for the same device (e.g. multiple EAS + // sessions racing a FolderSync reset after a hierarchy change such as + // moving contacts between address books) always reproduce the same UID + // for a given folder instead of a different random one each time. + // A random UID here made every reset diverge from the last one, + // which could cascade into devices with several concurrent + // Mail/Calendar/Contacts/Tasks/Notes sessions repeatedly invalidating + // each other's synckey and never settling. + // + // @author Torben Dannhauer + $this->_tempMap[$id] = sprintf('%s%08x', $prefix, crc32($prefix . ':' . $id)); $this->_logger->meta( sprintf( 'Creating new folder uuid for %s: %s', diff --git a/lib/Horde/ActiveSync/Request/MoveItems.php b/lib/Horde/ActiveSync/Request/MoveItems.php index af0849a4..ccaae7d1 100644 --- a/lib/Horde/ActiveSync/Request/MoveItems.php +++ b/lib/Horde/ActiveSync/Request/MoveItems.php @@ -115,8 +115,8 @@ protected function _handle() $status = self::STATUS_SAME_FOLDERS; } else { $importer = $this->_activeSync->getImporter(); - $importer->init($this->_state, $move[self::SRCFLDKEY]); try { + $importer->init($this->_state, $move[self::SRCFLDKEY]); $move_res = $importer->importMessageMove( [$move[self::SRCMSGKEY]], $move[self::DSTFLDKEY] @@ -133,6 +133,9 @@ protected function _handle() } else { $new_msgid = $move_res['results'][$move[self::SRCMSGKEY]]; } + } catch (Horde_ActiveSync_Exception_FolderGone $e) { + $this->_logger->err($e->getMessage()); + $status = self::STATUS_INVALID_SRC; } catch (Horde_ActiveSync_Exception $e) { $this->_logger->err($e->getMessage()); $status = self::STATUS_INVALID_SRC; diff --git a/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php b/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php new file mode 100644 index 00000000..9ae8a4ce --- /dev/null +++ b/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php @@ -0,0 +1,99 @@ + + * @license http://www.horde.org/licenses/gpl GPLv2 + * @copyright 2026 The Horde Project (http://www.horde.org/) + * @package ActiveSync + */ + +namespace Horde\ActiveSync; + +use Horde_ActiveSync; +use Horde_ActiveSync_Driver_Base; +use Horde_ActiveSync_State_Base; +use PHPUnit\Framework\Attributes\CoversNothing; +use PHPUnit\Framework\TestCase; +use ReflectionMethod; + +#[CoversNothing] +class FolderUidDeterministicTest extends TestCase +{ + public function testFolderUidIsDeterministicForRepeatedLookups() + { + $state = $this->createMock(Horde_ActiveSync_State_Base::class); + $state->method('getFolderUidToBackendIdMap')->willReturn([]); + + $driver = $this->getMockBuilder(Horde_ActiveSync_Driver_Base::class) + ->setConstructorArgs([['state' => $state]]) + ->getMockForAbstractClass(); + + $method = new ReflectionMethod(Horde_ActiveSync_Driver_Base::class, '_getFolderUidForBackendId'); + $method->setAccessible(true); + + $inbox = $method->invoke($driver, 'INBOX', Horde_ActiveSync::FOLDER_TYPE_INBOX); + $again = $method->invoke($driver, 'INBOX', Horde_ActiveSync::FOLDER_TYPE_INBOX); + + $this->assertSame($inbox, $again); + $this->assertSame( + 'F' . sprintf('%08x', crc32('F:' . 'INBOX')), + $inbox + ); + } + + public function testFolderUidIsDeterministicAcrossDriverInstances() + { + $state = $this->createMock(Horde_ActiveSync_State_Base::class); + $state->method('getFolderUidToBackendIdMap')->willReturn([]); + + $method = new ReflectionMethod(Horde_ActiveSync_Driver_Base::class, '_getFolderUidForBackendId'); + $method->setAccessible(true); + + $driverA = $this->getMockBuilder(Horde_ActiveSync_Driver_Base::class) + ->setConstructorArgs([['state' => $state]]) + ->getMockForAbstractClass(); + $driverB = $this->getMockBuilder(Horde_ActiveSync_Driver_Base::class) + ->setConstructorArgs([['state' => $state]]) + ->getMockForAbstractClass(); + + $contactBackendId = Horde_ActiveSync::CLASS_CONTACTS . ':42'; + $uidA = $method->invoke( + $driverA, + $contactBackendId, + Horde_ActiveSync::FOLDER_TYPE_USER_CONTACT + ); + $uidB = $method->invoke( + $driverB, + $contactBackendId, + Horde_ActiveSync::FOLDER_TYPE_USER_CONTACT + ); + + $this->assertSame($uidA, $uidB); + $this->assertSame( + 'C' . sprintf('%08x', crc32('C:' . $contactBackendId)), + $uidA + ); + } + + public function testFolderUidUsesPersistedMapWhenPresent() + { + $state = $this->createMock(Horde_ActiveSync_State_Base::class); + $state->method('getFolderUidToBackendIdMap')->willReturn([ + 'INBOX' => 'F85953279', + ]); + + $driver = $this->getMockBuilder(Horde_ActiveSync_Driver_Base::class) + ->setConstructorArgs([['state' => $state]]) + ->getMockForAbstractClass(); + + $method = new ReflectionMethod(Horde_ActiveSync_Driver_Base::class, '_getFolderUidForBackendId'); + $method->setAccessible(true); + + $this->assertSame( + 'F85953279', + $method->invoke($driver, 'INBOX', Horde_ActiveSync::FOLDER_TYPE_INBOX) + ); + } +} diff --git a/test/unit/Horde/ActiveSync/MoveItemsFolderGoneTest.php b/test/unit/Horde/ActiveSync/MoveItemsFolderGoneTest.php new file mode 100644 index 00000000..2a08efbc --- /dev/null +++ b/test/unit/Horde/ActiveSync/MoveItemsFolderGoneTest.php @@ -0,0 +1,224 @@ + + * @license http://www.horde.org/licenses/gpl GPLv2 + * @copyright 2026 The Horde Project (http://www.horde.org/) + * @package ActiveSync + */ + +namespace Horde\ActiveSync; + +use Horde_ActiveSync; +use Horde_ActiveSync_Connector_Importer; +use Horde_ActiveSync_Driver_Base; +use Horde_ActiveSync_Exception_FolderGone; +use Horde_ActiveSync_Log_Logger; +use Horde_ActiveSync_Request_MoveItems; +use Horde_ActiveSync_State_Base; +use Horde_ActiveSync_Wbxml; +use Horde_ActiveSync_Wbxml_Decoder; +use Horde_ActiveSync_Wbxml_Encoder; +use Horde_Controller_Request_Mock; +use Horde_Log_Handler_Null; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\TestCase; +use ReflectionMethod; +use ReflectionProperty; + +#[CoversClass(Horde_ActiveSync_Request_MoveItems::class)] +class MoveItemsFolderGoneTest extends TestCase +{ + protected function tearDown(): void + { + $deviceRef = new ReflectionProperty(Horde_ActiveSync::class, '_device'); + $deviceRef->setAccessible(true); + $deviceRef->setValue(null, null); + parent::tearDown(); + } + + public function testMoveItemsReturnsInvalidSrcWhenSourceFolderIsGone() + { + $fixture = $this->_createHandler(); + $fixture->importer->expects($this->once()) + ->method('init') + ->willThrowException(new Horde_ActiveSync_Exception_FolderGone('Folder not found in cache.')); + + $this->_writeWbxml($fixture->input, function (Horde_ActiveSync_Wbxml_Encoder $encoder) { + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::MOVES); + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::MOVE); + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::SRCMSGID); + $encoder->content('169462'); + $encoder->endTag(); + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::SRCFLDID); + $encoder->content('F85953279'); + $encoder->endTag(); + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::DSTFLDID); + $encoder->content('F37c56728'); + $encoder->endTag(); + $encoder->endTag(); + $encoder->endTag(); + }); + $fixture->decoder->readWbxmlHeader(); + + $method = new ReflectionMethod(Horde_ActiveSync_Request_MoveItems::class, '_handle'); + $method->setAccessible(true); + $this->assertTrue($method->invoke($fixture->handler)); + + $response = $this->_decodeMoveItemsResponse( + $this->_readStream($fixture->output) + ); + $this->assertSame(Horde_ActiveSync_Request_MoveItems::STATUS_INVALID_SRC, $response['status']); + $this->assertSame('169462', $response['srcMsgId']); + } + + public function testMoveItemsReturnsInvalidSrcWhenDestinationFolderIsGone() + { + $fixture = $this->_createHandler(); + $fixture->importer->expects($this->once())->method('init'); + $fixture->importer->expects($this->once()) + ->method('importMessageMove') + ->willThrowException(new Horde_ActiveSync_Exception_FolderGone('Folder not found in cache.')); + + $this->_writeWbxml($fixture->input, function (Horde_ActiveSync_Wbxml_Encoder $encoder) { + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::MOVES); + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::MOVE); + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::SRCMSGID); + $encoder->content('169462'); + $encoder->endTag(); + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::SRCFLDID); + $encoder->content('F85953279'); + $encoder->endTag(); + $encoder->startTag(Horde_ActiveSync_Request_MoveItems::DSTFLDID); + $encoder->content('F37c56728'); + $encoder->endTag(); + $encoder->endTag(); + $encoder->endTag(); + }); + $fixture->decoder->readWbxmlHeader(); + + $method = new ReflectionMethod(Horde_ActiveSync_Request_MoveItems::class, '_handle'); + $method->setAccessible(true); + $this->assertTrue($method->invoke($fixture->handler)); + + $response = $this->_decodeMoveItemsResponse( + $this->_readStream($fixture->output) + ); + $this->assertSame(Horde_ActiveSync_Request_MoveItems::STATUS_INVALID_SRC, $response['status']); + } + + /** + * @return object{ + * handler: Horde_ActiveSync_Request_MoveItems, + * input: resource, + * output: resource, + * importer: Horde_ActiveSync_Connector_Importer&\PHPUnit\Framework\MockObject\MockObject, + * state: Horde_ActiveSync_State_Base&\PHPUnit\Framework\MockObject\MockObject + * } + */ + protected function _createHandler(): object + { + $logger = new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null()); + $input = fopen('php://memory', 'wb+'); + $decoder = new Horde_ActiveSync_Wbxml_Decoder($input); + + $output = fopen('php://memory', 'wb+'); + $encoder = new Horde_ActiveSync_Wbxml_Encoder($output); + + $driver = $this->createMock(Horde_ActiveSync_Driver_Base::class); + $state = $this->createMock(Horde_ActiveSync_State_Base::class); + $importer = $this->createMock(Horde_ActiveSync_Connector_Importer::class); + + $request = new Horde_Controller_Request_Mock([ + 'get' => [ + 'Cmd' => 'MoveItems', + 'DeviceId' => 'TESTDEVICE', + ], + 'server' => [ + 'HTTP_MS_ASPROTOCOLVERSION' => Horde_ActiveSync::VERSION_SIXTEENONE, + ], + ]); + + $server = $this->getMockBuilder(Horde_ActiveSync::class) + ->setConstructorArgs([$driver, $decoder, $encoder, $state, $request]) + ->onlyMethods(['getImporter']) + ->getMock(); + $server->method('getImporter')->willReturn($importer); + + $device = new \Horde_ActiveSync_Device($state); + $device->id = 'TESTDEVICE'; + $device->version = Horde_ActiveSync::VERSION_SIXTEENONE; + $deviceRef = new ReflectionProperty(Horde_ActiveSync::class, '_device'); + $deviceRef->setAccessible(true); + $deviceRef->setValue(null, $device); + + $handler = new Horde_ActiveSync_Request_MoveItems($server); + $handler->setLogger($logger); + + return (object) [ + 'handler' => $handler, + 'decoder' => $decoder, + 'input' => $input, + 'output' => $output, + 'importer' => $importer, + 'state' => $state, + ]; + } + + protected function _writeWbxml($stream, callable $writer): void + { + $encoder = new Horde_ActiveSync_Wbxml_Encoder($stream); + $encoder->startWBXML(); + $writer($encoder); + rewind($stream); + } + + /** + * @return array{status: ?int, srcMsgId: ?string} + */ + protected function _decodeMoveItemsResponse(string $wbxml): array + { + $stream = fopen('php://memory', 'wb+'); + fwrite($stream, $wbxml); + rewind($stream); + + $decoder = new Horde_ActiveSync_Wbxml_Decoder($stream); + $decoder->readWbxmlHeader(); + $decoder->getElementStartTag(Horde_ActiveSync_Request_MoveItems::MOVES); + $decoder->getElementStartTag(Horde_ActiveSync_Request_MoveItems::RESPONSE); + + $response = [ + 'status' => null, + 'srcMsgId' => null, + ]; + + while (($child = $decoder->peek()) !== false + && $child[Horde_ActiveSync_Wbxml::EN_TYPE] === Horde_ActiveSync_Wbxml::EN_TYPE_STARTTAG + ) { + $tag = $child[Horde_ActiveSync_Wbxml::EN_TAG]; + $decoder->getElementStartTag($tag); + $content = $decoder->getElementContent(); + $decoder->getElementEndTag(); + + if ($tag === Horde_ActiveSync_Request_MoveItems::STATUS) { + $response['status'] = (int) $content; + } elseif ($tag === Horde_ActiveSync_Request_MoveItems::SRCMSGID) { + $response['srcMsgId'] = $content; + } + } + + $decoder->getElementEndTag(); + $decoder->getElementEndTag(); + fclose($stream); + + return $response; + } + + protected function _readStream($stream): string + { + rewind($stream); + return stream_get_contents($stream); + } +} From 8b9d15f80fade4800ead88cf05f1d1bb3dad6f24 Mon Sep 17 00:00:00 2001 From: Torben Dannhauer Date: Thu, 9 Jul 2026 23:17:48 +0200 Subject: [PATCH 2/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php b/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php index 9ae8a4ce..b74a1464 100644 --- a/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php +++ b/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php @@ -70,9 +70,8 @@ public function testFolderUidIsDeterministicAcrossDriverInstances() Horde_ActiveSync::FOLDER_TYPE_USER_CONTACT ); - $this->assertSame($uidA, $uidB); $this->assertSame( - 'C' . sprintf('%08x', crc32('C:' . $contactBackendId)), + 'C' . sprintf('%08x', crc32('C:' . $contactBackendId) & 0xffffffff), $uidA ); } From f399f3a85ea90ca94445a492b8ea01696fb21091 Mon Sep 17 00:00:00 2001 From: Torben Dannhauer Date: Thu, 9 Jul 2026 23:18:34 +0200 Subject: [PATCH 3/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php b/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php index b74a1464..5be44848 100644 --- a/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php +++ b/test/unit/Horde/ActiveSync/FolderUidDeterministicTest.php @@ -36,9 +36,8 @@ public function testFolderUidIsDeterministicForRepeatedLookups() $inbox = $method->invoke($driver, 'INBOX', Horde_ActiveSync::FOLDER_TYPE_INBOX); $again = $method->invoke($driver, 'INBOX', Horde_ActiveSync::FOLDER_TYPE_INBOX); - $this->assertSame($inbox, $again); $this->assertSame( - 'F' . sprintf('%08x', crc32('F:' . 'INBOX')), + 'F' . sprintf('%08x', crc32('F:' . 'INBOX') & 0xffffffff), $inbox ); } From 1580dd7fd60996c327be06ccacf0c6aaf72da3a8 Mon Sep 17 00:00:00 2001 From: Torben Dannhauer Date: Thu, 9 Jul 2026 23:19:05 +0200 Subject: [PATCH 4/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- lib/Horde/ActiveSync/Driver/Base.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Horde/ActiveSync/Driver/Base.php b/lib/Horde/ActiveSync/Driver/Base.php index 936d2737..7bd2bc41 100644 --- a/lib/Horde/ActiveSync/Driver/Base.php +++ b/lib/Horde/ActiveSync/Driver/Base.php @@ -424,9 +424,9 @@ protected function _getFolderUidForBackendId($id, $type = null, $old_id = null) // which could cascade into devices with several concurrent // Mail/Calendar/Contacts/Tasks/Notes sessions repeatedly invalidating // each other's synckey and never settling. - // // @author Torben Dannhauer - $this->_tempMap[$id] = sprintf('%s%08x', $prefix, crc32($prefix . ':' . $id)); + $crc = crc32($prefix . ':' . $id) & 0xffffffff; + $this->_tempMap[$id] = sprintf('%s%08x', $prefix, $crc); $this->_logger->meta( sprintf( 'Creating new folder uuid for %s: %s',