Skip to content

Feat/sync truncation3 #53976

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
kesselb merged 2 commits into from
Jul 17, 2025
Merged

Feat/sync truncation3 #53976

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a0ffc60
feat(carddav): handle truncated non-initial requests
kesselb Jun 17, 2025
1989027
fix(carddav): return correct sync token for non-truncated requests
kesselb Jun 17, 2025
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
Next Next commit
feat(carddav): handle truncated non-initial requests 
Signed-off-by: Daniel Kesselberg <[email protected]>
  • Loading branch information
@kesselb
kesselb committed Jul 17, 2025
commit a0ffc60c033932518c4b5a174223291cb4a9fe03
61 changes: 48 additions & 13 deletions apps/dav/lib/CardDAV/SyncService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Sabre\DAV\Xml\Response\MultiStatus;
use Sabre\DAV\Xml\Service;
use Sabre\VObject\Reader;
use Sabre\Xml\ParseException;
use function is_null;

class SyncService {
Expand All @@ -43,9 +44,10 @@ public function __construct(
}

/**
* @psalm-return list{0: ?string, 1: boolean}
* @throws \Exception
*/
public function syncRemoteAddressBook(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken, string $targetBookHash, string $targetPrincipal, array $targetProperties): string {
public function syncRemoteAddressBook(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken, string $targetBookHash, string $targetPrincipal, array $targetProperties): array {
// 1. create addressbook
$book = $this->ensureSystemAddressBookExists($targetPrincipal, $targetBookHash, $targetProperties);
$addressBookId = $book['id'];
Expand Down Expand Up @@ -83,7 +85,10 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add
}
}

return $response['token'];
return [
$response['token'],
$response['truncated'],
];
}

/**
Expand Down Expand Up @@ -127,7 +132,7 @@ public function ensureLocalSystemAddressBookExists(): ?array {

private function prepareUri(string $host, string $path): string {
/*
* The trailing slash is important for merging the uris together.
* The trailing slash is important for merging the uris.
*
* $host is stored in oc_trusted_servers.url and usually without a trailing slash.
*
Expand Down Expand Up @@ -158,7 +163,9 @@ private function prepareUri(string $host, string $path): string {
}

/**
* @return array{response: array<string, array<array-key, mixed>>, token: ?string, truncated: bool}
* @throws ClientExceptionInterface
* @throws ParseException
*/
protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array {
$client = $this->clientService->newClient();
Expand All @@ -181,7 +188,7 @@ protected function requestSyncReport(string $url, string $userName, string $addr
$body = $response->getBody();
assert(is_string($body));

return $this->parseMultiStatus($body);
return $this->parseMultiStatus($body, $addressBookUrl);
}

protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): string {
Expand Down Expand Up @@ -219,22 +226,50 @@ private function buildSyncCollectionRequestBody(?string $syncToken): string {
}

/**
* @param string $body
* @return array
* @throws \Sabre\Xml\ParseException
* @return array{response: array<string, array<array-key, mixed>>, token: ?string, truncated: bool}
* @throws ParseException
*/
private function parseMultiStatus($body) {
$xml = new Service();

private function parseMultiStatus(string $body, string $addressBookUrl): array {
/** @var MultiStatus $multiStatus */
$multiStatus = $xml->expect('{DAV:}multistatus', $body);
$multiStatus = (new Service())->expect('{DAV:}multistatus', $body);

$result = [];
$truncated = false;

foreach ($multiStatus->getResponses() as $response) {
$result[$response->getHref()] = $response->getResponseProperties();
$href = $response->getHref();
if ($response->getHttpStatus() === '507' && $this->isResponseForRequestUri($href, $addressBookUrl)) {
$truncated = true;
} else {
$result[$response->getHref()] = $response->getResponseProperties();
}
}

return ['response' => $result, 'token' => $multiStatus->getSyncToken()];
return ['response' => $result, 'token' => $multiStatus->getSyncToken(), 'truncated' => $truncated];
}

/**
* Determines whether the provided response URI corresponds to the given request URI.
*/
private function isResponseForRequestUri(string $responseUri, string $requestUri): bool {
/*
* Example response uri:
*
* /remote.php/dav/addressbooks/system/system/system/
* /cloud/remote.php/dav/addressbooks/system/system/system/ (when installed in a subdirectory)
*
* Example request uri:
*
* remote.php/dav/addressbooks/system/system/system
*
* References:
* https://github.com/nextcloud/3rdparty/blob/e0a509739b13820f0a62ff9cad5d0fede00e76ee/sabre/dav/lib/DAV/Sync/Plugin.php#L172-L174
* https://github.com/nextcloud/server/blob/b40acb34a39592070d8455eb91c5364c07928c50/apps/federation/lib/SyncFederationAddressBooks.php#L41
*/
return str_ends_with(
rtrim($responseUri, '/'),
rtrim($requestUri, '/')
);
}

/**
Expand Down
29 changes: 19 additions & 10 deletions apps/federation/lib/SyncFederationAddressBooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function syncThemAll(\Closure $callback) {
$url = $trustedServer['url'];
$callback($url, null);
$sharedSecret = $trustedServer['shared_secret'];
$syncToken = $trustedServer['sync_token'];
$oldSyncToken = $trustedServer['sync_token'];

$endPoints = $this->ocsDiscoveryService->discover($url, 'FEDERATED_SHARING');
$cardDavUser = $endPoints['carddav-user'] ?? 'system';
Expand All @@ -49,16 +49,25 @@ public function syncThemAll(\Closure $callback) {
$targetBookProperties = [
'{DAV:}displayname' => $url
];

try {
$newToken = $this->syncService->syncRemoteAddressBook($url, $cardDavUser, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties);
if ($newToken !== $syncToken) {
// Finish truncated initial sync.
if (strpos($newToken, 'init') !== false) {
do {
$newToken = $this->syncService->syncRemoteAddressBook($url, $cardDavUser, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties);
} while (str_contains($newToken, 'init_'));
}
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $newToken);
$syncToken = $oldSyncToken;

do {
[$syncToken, $truncated] = $this->syncService->syncRemoteAddressBook(
$url,
$cardDavUser,
$addressBookUrl,
$sharedSecret,
$syncToken,
$targetBookId,
$targetPrincipal,
$targetBookProperties
);
} while ($truncated);

if ($syncToken !== $oldSyncToken) {
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $syncToken);
} else {
$this->logger->debug("Sync Token for $url unchanged from previous sync");
// The server status might have been changed to a failure status in previous runs.
Expand Down