From 066b42fcffee9c689c428dc1aa27fb951704e963 Mon Sep 17 00:00:00 2001 From: Torben Dannhauer Date: Fri, 17 Jul 2026 18:11:09 +0200 Subject: [PATCH 1/2] fix(activesync): refuse Draft Modify append-as-new for stale UIDs When an EAS 16 Drafts Modify references an IMAP UID that is already gone, do not treat it as a new draft append. Retries after a lost Sync response would otherwise grow the Drafts folder unbounded. Defense in depth for horde/ActiveSync#85 (idempotent importer short-circuit). --- doc/changelog.yml | 10 ++ lib/Horde/Core/ActiveSync/Driver.php | 23 +++- .../DriverDraftModifyStaleUidTest.php | 118 ++++++++++++++++++ 3 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 test/Unit/ActiveSync/DriverDraftModifyStaleUidTest.php diff --git a/doc/changelog.yml b/doc/changelog.yml index 8d10a158..ebe4ae5a 100644 --- a/doc/changelog.yml +++ b/doc/changelog.yml @@ -1,4 +1,14 @@ --- +3.2.1: + api: 3.2.1 + state: + release: stable + api: stable + date: 2026-07-17 + license: + identifier: LGPL-2.1-only + uri: https://spdx.org/licenses/LGPL-2.1-only.html + notes: "fix(activesync): refuse Draft Modify append-as-new for missing/stale IMAP UIDs (ActiveSync #85)" 3.2.0: api: 3.2.0 state: diff --git a/lib/Horde/Core/ActiveSync/Driver.php b/lib/Horde/Core/ActiveSync/Driver.php index 4335e32c..70b883d5 100644 --- a/lib/Horde/Core/ActiveSync/Driver.php +++ b/lib/Horde/Core/ActiveSync/Driver.php @@ -2565,8 +2565,10 @@ public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $mes $this->_user, $this->_version ); - $draft->setDraftMessage($message); + // Load the existing draft before setDraftMessage so + // attachment carry-over works, and so a missing/ + // stale Modify ServerId fails before MIME build. if ($id) { try { $draft->getExistingDraftMessage( @@ -2574,10 +2576,26 @@ public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $mes $id ); } catch (Horde_ActiveSync_Exception $e) { - // Stale UID from client; treat as new. + // Modify with a missing/stale UID must not + // append a new draft (retries after a lost + // Sync response would otherwise duplicate + // unbounded). Importer short-circuits when + // the prior apply was recorded; this is + // defense in depth when that record is absent. + $this->_logger->notice( + sprintf( + 'Draft Modify for missing UID %s in %s; refusing append-as-new.', + $id, + $draft_folder + ) + ); + $this->_endBuffer(); + return false; } } + $draft->setDraftMessage($message); + // Append the message and return results. $results = $draft->append($draft_folder); $stat['id'] = $results['uid']; @@ -2585,6 +2603,7 @@ public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $mes $stat['conversationid'] = bin2hex($message->subject); $stat['conversationindex'] = time(); + $this->_endBuffer(); return $stat; } } diff --git a/test/Unit/ActiveSync/DriverDraftModifyStaleUidTest.php b/test/Unit/ActiveSync/DriverDraftModifyStaleUidTest.php new file mode 100644 index 00000000..5a07478c --- /dev/null +++ b/test/Unit/ActiveSync/DriverDraftModifyStaleUidTest.php @@ -0,0 +1,118 @@ + + * @category Horde + * @copyright 2026 The Horde Project + * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 + * @package Core + * @subpackage UnitTests + */ + +namespace Horde\Core\Test\Unit\ActiveSync; + +use Horde\Http\ServerRequest; +use Horde_ActiveSync; +use Horde_ActiveSync_Device; +use Horde_ActiveSync_Imap_Adapter; +use Horde_ActiveSync_Message_AirSyncBaseBody; +use Horde_ActiveSync_Message_Mail; +use Horde_Core_ActiveSync_Auth; +use Horde_Core_ActiveSync_Connector; +use Horde_Core_ActiveSync_Driver; +use Horde_Log_Handler_Null; +use Horde_Log_Logger; +use Horde_Registry; +use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\TestCase; + +/** + * Defense-in-depth for Issue #85: Draft Modify must not append-as-new when + * the client ServerId is missing/stale on IMAP. + */ +#[CoversMethod(Horde_Core_ActiveSync_Driver::class, 'changeMessage')] +class DriverDraftModifyStaleUidTest extends TestCase +{ + public function testDraftModifyMissingUidDoesNotAppend(): void + { + if (!class_exists('Horde_ActiveSync_Message_Mail')) { + $this->markTestSkipped('horde/activesync not available'); + } + + $folder = 'INBOX/Drafts'; + $staleUid = 100; + + $imap = $this->getMockBuilder(Horde_ActiveSync_Imap_Adapter::class) + ->disableOriginalConstructor() + ->onlyMethods(['getImapMessage', 'appendMessage', 'deleteMessages', 'getSpecialMailboxes', 'setLogger']) + ->getMock(); + $imap->method('getSpecialMailboxes')->willReturn([ + Horde_Core_ActiveSync_Driver::SPECIAL_DRAFTS => (object) ['value' => $folder], + ]); + $imap->expects($this->once()) + ->method('getImapMessage') + ->with($folder, $staleUid) + ->willReturn([]); + $imap->expects($this->never())->method('appendMessage'); + $imap->expects($this->never())->method('deleteMessages'); + + $state = $this->getMockBuilder('Horde_ActiveSync_State_Sql') + ->disableOriginalConstructor() + ->getMock(); + $state->method('setLogger'); + $state->method('setBackend'); + + $connector = $this->getMockBuilder(Horde_Core_ActiveSync_Connector::class) + ->disableOriginalConstructor() + ->getMock(); + $auth = $this->getMockBuilder(Horde_Core_ActiveSync_Auth::class) + ->disableOriginalConstructor() + ->getMock(); + $registry = $this->getMockBuilder(Horde_Registry::class) + ->disableOriginalConstructor() + ->getMock(); + + $driver = new Horde_Core_ActiveSync_Driver([ + 'connector' => $connector, + 'auth' => $auth, + 'serverrequest' => new ServerRequest('POST', '/'), + 'registry' => $registry, + 'state' => $state, + 'imap' => $imap, + ]); + $driver->setLogger(new Horde_Log_Logger(new Horde_Log_Handler_Null())); + $driver->setProtocolVersion(Horde_ActiveSync::VERSION_SIXTEEN); + + $userProp = new \ReflectionProperty( + \Horde_ActiveSync_Driver_Base::class, + '_user' + ); + $userProp->setAccessible(true); + $userProp->setValue($driver, 'alice@example.com'); + + $message = new Horde_ActiveSync_Message_Mail([ + 'protocolversion' => Horde_ActiveSync::VERSION_SIXTEEN, + ]); + $message->to = 'alice@example.com'; + $message->subject = 'Draft subject'; + $body = new Horde_ActiveSync_Message_AirSyncBaseBody([ + 'protocolversion' => Horde_ActiveSync::VERSION_SIXTEEN, + ]); + $body->type = Horde_ActiveSync::BODYPREF_TYPE_PLAIN; + $body->data = 'body'; + $message->airsyncbasebody = $body; + + $device = $this->createMock(Horde_ActiveSync_Device::class); + + $result = $driver->changeMessage($folder, $staleUid, $message, $device); + + $this->assertFalse($result); + } +} From ffbd4895d6055f6207b59b594ba34620a93a196a Mon Sep 17 00:00:00 2001 From: Torben Dannhauer Date: Sat, 18 Jul 2026 11:29:02 +0200 Subject: [PATCH 2/2] fix(core): address PR review for draft Modify stale UID Drop the hand-edited changelog entry (release tool owns it) and use import statements in the unit test instead of inline FQCNs. --- doc/changelog.yml | 10 ---------- test/Unit/ActiveSync/DriverDraftModifyStaleUidTest.php | 6 ++++-- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/doc/changelog.yml b/doc/changelog.yml index ebe4ae5a..8d10a158 100644 --- a/doc/changelog.yml +++ b/doc/changelog.yml @@ -1,14 +1,4 @@ --- -3.2.1: - api: 3.2.1 - state: - release: stable - api: stable - date: 2026-07-17 - license: - identifier: LGPL-2.1-only - uri: https://spdx.org/licenses/LGPL-2.1-only.html - notes: "fix(activesync): refuse Draft Modify append-as-new for missing/stale IMAP UIDs (ActiveSync #85)" 3.2.0: api: 3.2.0 state: diff --git a/test/Unit/ActiveSync/DriverDraftModifyStaleUidTest.php b/test/Unit/ActiveSync/DriverDraftModifyStaleUidTest.php index 5a07478c..00bb79d8 100644 --- a/test/Unit/ActiveSync/DriverDraftModifyStaleUidTest.php +++ b/test/Unit/ActiveSync/DriverDraftModifyStaleUidTest.php @@ -21,6 +21,7 @@ use Horde\Http\ServerRequest; use Horde_ActiveSync; use Horde_ActiveSync_Device; +use Horde_ActiveSync_Driver_Base; use Horde_ActiveSync_Imap_Adapter; use Horde_ActiveSync_Message_AirSyncBaseBody; use Horde_ActiveSync_Message_Mail; @@ -32,6 +33,7 @@ use Horde_Registry; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\TestCase; +use ReflectionProperty; /** * Defense-in-depth for Issue #85: Draft Modify must not append-as-new when @@ -90,8 +92,8 @@ public function testDraftModifyMissingUidDoesNotAppend(): void $driver->setLogger(new Horde_Log_Logger(new Horde_Log_Handler_Null())); $driver->setProtocolVersion(Horde_ActiveSync::VERSION_SIXTEEN); - $userProp = new \ReflectionProperty( - \Horde_ActiveSync_Driver_Base::class, + $userProp = new ReflectionProperty( + Horde_ActiveSync_Driver_Base::class, '_user' ); $userProp->setAccessible(true);