feat(carddav): handle truncated non-initial requestsfeat/sync-truncation2

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

2 files changed, 67 insertions, 23 deletions

diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php
index c31a70b2aa5..8659c6db5dc 100644
--- a/apps/dav/lib/CardDAV/SyncService.php
+++ b/apps/dav/lib/CardDAV/SyncService.php
@@ -22,6 +22,7 @@ use Psr\Log\LoggerInterface;
 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 {
@@ -43,9 +44,10 @@ class SyncService {
 	}
 
 	/**
+	 * @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'];
@@ -83,7 +85,10 @@ class SyncService {
 			}
 		}
 
-		return $response['token'];
+		return [
+			$response['token'],
+			$response['truncated'],
+		];
 	}
 
 	/**
@@ -127,7 +132,7 @@ class SyncService {
 
 	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.
 		 *
@@ -158,7 +163,9 @@ class SyncService {
 	}
 
 	/**
+	 * @return array{response: array{string, array<array-key, mixed}, token: ?string, truncated: boolean}
 	 * @throws ClientExceptionInterface
+	 * @throws ParseException
 	 */
 	protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array {
 		$client = $this->clientService->newClient();
@@ -180,7 +187,7 @@ class SyncService {
 		$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 {
@@ -217,22 +224,50 @@ class SyncService {
 	}
 
 	/**
-	 * @param string $body
-	 * @return array
-	 * @throws \Sabre\Xml\ParseException
+	 * @return array{response: array{string, array<array-key, mixed}, token: ?string, truncated: boolean}
+	 * @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, '/')
+		);
 	}
 
 	/**
diff --git a/apps/federation/lib/SyncFederationAddressBooks.php b/apps/federation/lib/SyncFederationAddressBooks.php
index 7f700cde21b..d11f92b76ef 100644
--- a/apps/federation/lib/SyncFederationAddressBooks.php
+++ b/apps/federation/lib/SyncFederationAddressBooks.php
@@ -34,7 +34,7 @@ class SyncFederationAddressBooks {
 			$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';
@@ -49,16 +49,25 @@ class SyncFederationAddressBooks {
 			$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.