Skip to content

fix(core): report rejected client data as ActiveSync Status 6 #248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ralflang merged 1 commit into from
Nov 2, 2026
Merged

fix(core): report rejected client data as ActiveSync Status 6 #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 47 additions & 24 deletions lib/Horde/Core/ActiveSync/Driver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

use Horde\Exception\InvalidDataThrowable;
use Psr\Http\Message\ServerRequestInterface;

/**
Expand Down Expand Up @@ -2478,6 +2479,11 @@ public function moveMessage($folderid, array $ids, $newfolderid)
* if adding Draft email message. @since 2.28.0
* - conversationindex: (integer) The conversation index value if adding
* Draft email message @since 2.28.0
*
* On failure: false for a server-side error the client may retry, or
* an array with only an 'error' key holding the
* Horde_ActiveSync_Request_Sync::STATUS_* code to report for the item
* (STATUS_INVALID when the backend rejected the client data).
*/
public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $message, $device)
{
Expand Down Expand Up @@ -2513,9 +2519,7 @@ public function changeMessage($folderid, $id, Horde_ActiveSync_Message_Base $mes
try {
$results = $this->_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),
Expand All @@ -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.
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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.
*
Expand Down
131 changes: 131 additions & 0 deletions test/Unit/ActiveSync/DriverChangeMessageStatusTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
<?php

declare(strict_types=1);

/**
* Copyright 2026 The Horde Project (http://www.horde.org/)
*
* See the enclosed file LICENSE for license information (LGPL). If you
* did not receive this file, see http://www.horde.org/licenses/lgpl21.
*
* @author Torben Dannhauer <torben@dannhauer.de>
* @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,
]);
}
}
Loading