diff --git a/lib/Horde/Core/ActiveSync/Driver.php b/lib/Horde/Core/ActiveSync/Driver.php index e0c70fc6..25ade46b 100644 --- a/lib/Horde/Core/ActiveSync/Driver.php +++ b/lib/Horde/Core/ActiveSync/Driver.php @@ -1,5 +1,6 @@ _connector->calendar_import($message, $server_id); } catch (Horde_Exception $e) { - $this->_logger->err($e->getMessage()); - $this->_endBuffer(); - return false; + return $this->_importFailure($e, $folderid, $id); } $stat = [ 'mod' => $this->getSyncStamp($folderid), @@ -2530,9 +2534,7 @@ public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $mes try { $results = $this->_connector->calendar_replace($id, $message, $server_id); } catch (Horde_Exception $e) { - $this->_logger->err($e->getMessage()); - $this->_endBuffer(); - return false; + return $this->_importFailure($e, $folderid, $id); } $stat = $this->_smartStatMessage($folderid, $id, false); // @todo Remove this check in H6, when the API always returns. @@ -2549,18 +2551,14 @@ public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $mes try { $id = $this->_connector->contacts_import($message, $server_id); } catch (Horde_Exception $e) { - $this->_logger->err($e->getMessage()); - $this->_endBuffer(); - return false; + return $this->_importFailure($e, $folderid, $id); } $stat = ['mod' => $this->getSyncStamp($folderid), 'id' => $id, 'flags' => 1]; } else { try { $this->_connector->contacts_replace($id, $message); } catch (Horde_Exception $e) { - $this->_logger->err($e->getMessage()); - $this->_endBuffer(); - return false; + return $this->_importFailure($e, $folderid, $id); } $stat = $this->_smartStatMessage($folderid, $id, false); } @@ -2571,18 +2569,14 @@ public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $mes try { $id = $this->_connector->tasks_import($message, $server_id); } catch (Horde_Exception $e) { - $this->_logger->err($e->getMessage()); - $this->_endBuffer(); - return false; + return $this->_importFailure($e, $folderid, $id); } $stat = ['mod' => $this->getSyncStamp($folderid), 'id' => $id, 'flags' => 1]; } else { try { $this->_connector->tasks_replace($id, $message); } catch (Horde_Exception $e) { - $this->_logger->err($e->getMessage()); - $this->_endBuffer(); - return false; + return $this->_importFailure($e, $folderid, $id); } $stat = $this->_smartStatMessage($folderid, $id, false); } @@ -2593,18 +2587,14 @@ public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $mes try { $id = $this->_connector->notes_import($message, $server_id); } catch (Horde_Exception $e) { - $this->_logger->err($e->getMessage()); - $this->_endBuffer(); - return false; + return $this->_importFailure($e, $folderid, $id); } $stat = ['mod' => $this->getSyncStamp($folderid), 'id' => $id, 'flags' => 1]; } else { try { $this->_connector->notes_replace($id, $message); } catch (Horde_Exception $e) { - $this->_logger->err($e->getMessage()); - $this->_endBuffer(); - return false; + return $this->_importFailure($e, $folderid, $id); } $stat = $this->_smartStatMessage($folderid, $id, false); } @@ -4661,6 +4651,39 @@ protected function _endBuffer() } } + /** + * Turn an exception raised by a PIM backend while importing a client + * change into the changeMessage() failure result. + * + * A backend that rejected the client's data (InvalidDataThrowable, e.g. + * an attribute over its configured length) is reported as STATUS_INVALID: + * the item itself is unacceptable and the client must not resend it on + * every Sync. Any other failure stays a generic server error (false), + * which the client may retry. + * + * @param Horde_Exception $e The backend exception. + * @param string $folderid The folder the change was addressed to. + * @param string|null $id The server id of the changed item, null for + * an addition. + * + * @return array|boolean @see self::changeMessage() + */ + protected function _importFailure(Horde_Exception $e, $folderid, $id) + { + $this->_endBuffer(); + if ($e instanceof InvalidDataThrowable) { + $this->_logger->warn(sprintf( + 'Rejected client change for %s in folder %s: %s', + $id ?: 'new item', + $folderid, + $e->getMessage() + )); + return ['error' => Horde_ActiveSync_Request_Sync::STATUS_INVALID]; + } + $this->_logger->err($e->getMessage()); + return false; + } + /** * Removes the beginning/ending delimiters from the certificate. * diff --git a/test/Unit/ActiveSync/DriverChangeMessageStatusTest.php b/test/Unit/ActiveSync/DriverChangeMessageStatusTest.php new file mode 100644 index 00000000..3c25593f --- /dev/null +++ b/test/Unit/ActiveSync/DriverChangeMessageStatusTest.php @@ -0,0 +1,131 @@ + + * @category Horde + * @copyright 2026 The Horde Project + * @license http://www.horde.org/licenses/lgpl21 LGPL + * @package Core + * @subpackage UnitTests + */ + +namespace Horde\Core\Test\Unit\ActiveSync; + +use Horde\Core\Test\Support\MockSkipConstructorTrait; +use Horde\Exception\InvalidDataThrowable; +use Horde\Http\ServerRequest; +use Horde_ActiveSync_Message_Contact; +use Horde_ActiveSync_Request_Sync; +use Horde_Core_ActiveSync_Auth; +use Horde_Core_ActiveSync_Connector; +use Horde_Core_ActiveSync_Driver; +use Horde_Exception; +use Horde_Registry; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\TestCase; + +/** + * Unit tests for the failure result of + * Horde_Core_ActiveSync_Driver::changeMessage(). + * + * A PIM backend that rejects the client's data (InvalidDataThrowable, e.g. + * Turba refusing an attribute over its configured length) must be reported + * as STATUS_INVALID so the client stops resending the item; any other + * backend failure stays a retryable false. + */ +#[CoversClass(Horde_Core_ActiveSync_Driver::class)] +class DriverChangeMessageStatusTest extends TestCase +{ + use MockSkipConstructorTrait; + + public function testRejectedClientDataIsReportedAsStatusInvalid(): void + { + $connector = $this->getMockSkipConstructor(Horde_Core_ActiveSync_Connector::class); + $connector->expects($this->once()) + ->method('contacts_replace') + ->willThrowException($this->invalidDataException()); + + $result = $this->driver($connector)->changeMessage( + Horde_Core_ActiveSync_Driver::CONTACTS_FOLDER_UID, + 'contact-1', + new Horde_ActiveSync_Message_Contact(), + null + ); + + $this->assertSame( + ['error' => Horde_ActiveSync_Request_Sync::STATUS_INVALID], + $result + ); + } + + public function testRejectedClientDataOnAddIsReportedAsStatusInvalid(): void + { + $connector = $this->getMockSkipConstructor(Horde_Core_ActiveSync_Connector::class); + $connector->expects($this->once()) + ->method('contacts_import') + ->willThrowException($this->invalidDataException()); + + $result = $this->driver($connector)->changeMessage( + Horde_Core_ActiveSync_Driver::CONTACTS_FOLDER_UID, + null, + new Horde_ActiveSync_Message_Contact(), + null + ); + + $this->assertSame( + ['error' => Horde_ActiveSync_Request_Sync::STATUS_INVALID], + $result + ); + } + + public function testOtherBackendFailureStaysRetryableFalse(): void + { + $connector = $this->getMockSkipConstructor(Horde_Core_ActiveSync_Connector::class); + $connector->expects($this->once()) + ->method('contacts_replace') + ->willThrowException(new Horde_Exception('Server error when saving data.')); + + $result = $this->driver($connector)->changeMessage( + Horde_Core_ActiveSync_Driver::CONTACTS_FOLDER_UID, + 'contact-1', + new Horde_ActiveSync_Message_Contact(), + null + ); + + $this->assertFalse($result); + } + + private function invalidDataException(): Horde_Exception + { + return new class ('Home Postal Code: Value is over the maximum length of 10.') extends Horde_Exception implements InvalidDataThrowable { + }; + } + + private function driver(Horde_Core_ActiveSync_Connector $connector): Horde_Core_ActiveSync_Driver + { + if (!class_exists('Horde_ActiveSync_State_Sql')) { + $this->markTestSkipped('horde/activesync not available'); + } + + $state = $this->getMockSkipConstructor('Horde_ActiveSync_State_Sql'); + $auth = $this->getMockSkipConstructor(Horde_Core_ActiveSync_Auth::class); + $auth->expects($this->never())->method($this->anything()); + $registry = $this->getMockSkipConstructor(Horde_Registry::class); + $registry->expects($this->never())->method($this->anything()); + + return new Horde_Core_ActiveSync_Driver([ + 'connector' => $connector, + 'auth' => $auth, + 'serverrequest' => new ServerRequest('POST', '/'), + 'registry' => $registry, + 'state' => $state, + ]); + } +}