refactor(dav): migrate to new http client

The request method is available since 29 and thus we can finally use the modern http client to send the report request for the addressbook sync.

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

1 files changed, 41 insertions, 44 deletions

diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php
index 01747a9b105..c32704b30d2 100644
--- a/apps/dav/lib/CardDAV/SyncService.php
+++ b/apps/dav/lib/CardDAV/SyncService.php
@@ -32,14 +32,14 @@ namespace OCA\DAV\CardDAV;
 
 use OCP\AppFramework\Db\TTransactional;
 use OCP\AppFramework\Http;
+use OCP\Http\Client\IClientService;
 use OCP\IDBConnection;
 use OCP\IUser;
 use OCP\IUserManager;
+use Psr\Http\Client\ClientExceptionInterface;
 use Psr\Log\LoggerInterface;
-use Sabre\DAV\Client;
 use Sabre\DAV\Xml\Response\MultiStatus;
 use Sabre\DAV\Xml\Service;
-use Sabre\HTTP\ClientHttpException;
 use Sabre\VObject\Reader;
 use function is_null;
 
@@ -54,18 +54,21 @@ class SyncService {
 	private ?array $localSystemAddressBook = null;
 	private Converter $converter;
 	protected string $certPath;
+	private IClientService $clientService;
 
 	public function __construct(CardDavBackend $backend,
 		IUserManager $userManager,
 		IDBConnection $dbConnection,
 		LoggerInterface $logger,
-		Converter $converter) {
+		Converter $converter,
+		IClientService $clientService) {
 		$this->backend = $backend;
 		$this->userManager = $userManager;
 		$this->logger = $logger;
 		$this->converter = $converter;
 		$this->certPath = '';
 		$this->dbConnection = $dbConnection;
+		$this->clientService = $clientService;
 	}
 
 	/**
@@ -79,7 +82,7 @@ class SyncService {
 		// 2. query changes
 		try {
 			$response = $this->requestSyncReport($url, $userName, $addressBookUrl, $sharedSecret, $syncToken);
-		} catch (ClientHttpException $ex) {
+		} catch (ClientExceptionInterface $ex) {
 			if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) {
 				// remote server revoked access to the address book, remove it
 				$this->backend->deleteAddressBook($addressBookId);
@@ -99,9 +102,9 @@ class SyncService {
 				$this->atomic(function () use ($addressBookId, $cardUri, $vCard) {
 					$existingCard = $this->backend->getCard($addressBookId, $cardUri);
 					if ($existingCard === false) {
-						$this->backend->createCard($addressBookId, $cardUri, $vCard['body']);
+						$this->backend->createCard($addressBookId, $cardUri, $vCard);
 					} else {
-						$this->backend->updateCard($addressBookId, $cardUri, $vCard['body']);
+						$this->backend->updateCard($addressBookId, $cardUri, $vCard);
 					}
 				}, $this->dbConnection);
 			} else {
@@ -128,56 +131,50 @@ class SyncService {
 	}
 
 	/**
-	 * Check if there is a valid certPath we should use
+	 * @throws ClientExceptionInterface
 	 */
-	protected function getCertPath(): string {
-
-		// we already have a valid certPath
-		if ($this->certPath !== '') {
-			return $this->certPath;
-		}
-
-		$certManager = \OC::$server->getCertificateManager();
-		$certPath = $certManager->getAbsoluteBundlePath();
-		if (file_exists($certPath)) {
-			$this->certPath = $certPath;
-		}
+	protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array {
+		$client = $this->clientService->newClient();
 
-		return $this->certPath;
-	}
+		// the trailing slash is important for merging base_uri and uri
+		$url = rtrim($url, '/') . '/';
 
-	protected function getClient(string $url, string $userName, string $sharedSecret): Client {
-		$settings = [
-			'baseUri' => $url . '/',
-			'userName' => $userName,
-			'password' => $sharedSecret,
+		$options = [
+			'auth' => [$userName, $sharedSecret],
+			'base_uri' => $url,
+			'body' => $this->buildSyncCollectionRequestBody($syncToken),
+			'headers' => ['Content-Type' => 'application/xml']
 		];
-		$client = new Client($settings);
-		$certPath = $this->getCertPath();
-		$client->setThrowExceptions(true);
 
-		if ($certPath !== '' && !str_starts_with($url, 'http://')) {
-			$client->addCurlSetting(CURLOPT_CAINFO, $this->certPath);
-		}
+		$response = $client->request(
+			'REPORT',
+			$addressBookUrl,
+			$options
+		);
+
+		$body = $response->getBody();
+		assert(is_string($body));
 
-		return $client;
+		return $this->parseMultiStatus($body);
 	}
 
-	protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array {
-		$client = $this->getClient($url, $userName, $sharedSecret);
+	protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): string {
+		$client = $this->clientService->newClient();
 
-		$body = $this->buildSyncCollectionRequestBody($syncToken);
+		// the trailing slash is important for merging base_uri and uri
+		$url = rtrim($url, '/') . '/';
 
-		$response = $client->request('REPORT', $addressBookUrl, $body, [
-			'Content-Type' => 'application/xml'
-		]);
+		$options = [
+			'auth' => [$userName, $sharedSecret],
+			'base_uri' => $url,
+		];
 
-		return $this->parseMultiStatus($response['body']);
-	}
+		$response = $client->get(
+			$resourcePath,
+			$options
+		);
 
-	protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): array {
-		$client = $this->getClient($url, $userName, $sharedSecret);
-		return $client->request('GET', $resourcePath);
+		return (string)$response->getBody();
 	}
 
 	private function buildSyncCollectionRequestBody(?string $syncToken): string {