From 9c594cda20903b2035bbfeae171e69aa445ad6d0 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Tue, 1 Oct 2024 22:52:08 +0200 Subject: [PATCH] fix(caldav): add missing handlers Signed-off-by: Anna Larch --- .../WebcalCaching/RefreshWebcalService.php | 151 +++++++----------- .../RefreshWebcalServiceTest.php | 31 ++-- 2 files changed, 71 insertions(+), 111 deletions(-) diff --git a/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php b/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php index 8928d05b93c96..ddee241f52866 100644 --- a/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php +++ b/apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php @@ -31,18 +31,14 @@ namespace OCA\DAV\CalDAV\WebcalCaching; use Exception; -use GuzzleHttp\HandlerStack; -use GuzzleHttp\Middleware; +use GuzzleHttp\RequestOptions; use OCA\DAV\CalDAV\CalDavBackend; use OCP\Http\Client\IClientService; use OCP\Http\Client\LocalServerException; use OCP\IConfig; -use Psr\Http\Message\RequestInterface; -use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\PropPatch; -use Sabre\DAV\Xml\Property\Href; use Sabre\VObject\Component; use Sabre\VObject\DateTimeParser; use Sabre\VObject\InvalidDataException; @@ -76,7 +72,7 @@ public function __construct(CalDavBackend $calDavBackend, IClientService $client $this->logger = $logger; } - public function refreshSubscription(string $principalUri, string $uri) { + public function refreshSubscription(string $principalUri, string $uri): void { $subscription = $this->getSubscription($principalUri, $uri); $mutations = []; if (!$subscription) { @@ -84,7 +80,7 @@ public function refreshSubscription(string $principalUri, string $uri) { } $webcalData = $this->queryWebcalFeed($subscription, $mutations); - if (!$webcalData) { + if ($webcalData === null) { return; } @@ -127,7 +123,7 @@ public function refreshSubscription(string $principalUri, string $uri) { $calendarData = $vObject->serialize(); try { $this->calDavBackend->createCalendarObject($subscription['id'], $objectUri, $calendarData, CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION); - } catch (NoInstancesException | BadRequest $ex) { + } catch (NoInstancesException|BadRequest $ex) { $this->logger->error('Unable to create calendar object from subscription {subscriptionId}', ['exception' => $ex, 'subscriptionId' => $subscription['id'], 'source' => $subscription['source']]); } } @@ -139,7 +135,7 @@ public function refreshSubscription(string $principalUri, string $uri) { $this->updateSubscription($subscription, $mutations); } catch (ParseException $ex) { - $this->logger->error("Subscription {subscriptionId} could not be refreshed due to a parsing error", ['exception' => $ex, 'subscriptionId' => $subscription['id']]); + $this->logger->error('Subscription {subscriptionId} could not be refreshed due to a parsing error', ['exception' => $ex, 'subscriptionId' => $subscription['id']]); } } @@ -165,106 +161,81 @@ function ($sub) use ($uri) { * gets webcal feed from remote server */ private function queryWebcalFeed(array $subscription, array &$mutations): ?string { - $client = $this->clientService->newClient(); - - $didBreak301Chain = false; - $latestLocation = null; - - $handlerStack = HandlerStack::create(); - $handlerStack->push(Middleware::mapRequest(function (RequestInterface $request) { - return $request - ->withHeader('Accept', 'text/calendar, application/calendar+json, application/calendar+xml') - ->withHeader('User-Agent', 'Nextcloud Webcal Service'); - })); - $handlerStack->push(Middleware::mapResponse(function (ResponseInterface $response) use (&$didBreak301Chain, &$latestLocation) { - if (!$didBreak301Chain) { - if ($response->getStatusCode() !== 301) { - $didBreak301Chain = true; - } else { - $latestLocation = $response->getHeader('Location'); - } - } - return $response; - })); - - $allowLocalAccess = $this->config->getAppValue('dav', 'webcalAllowLocalAccess', 'no'); $subscriptionId = $subscription['id']; $url = $this->cleanURL($subscription['source']); if ($url === null) { return null; } - try { - $params = [ - 'allow_redirects' => [ - 'redirects' => 10 - ], - 'handler' => $handlerStack, - 'nextcloud' => [ - 'allow_local_address' => $allowLocalAccess === 'yes', - ] - ]; - - $user = parse_url($subscription['source'], PHP_URL_USER); - $pass = parse_url($subscription['source'], PHP_URL_PASS); - if ($user !== null && $pass !== null) { - $params['auth'] = [$user, $pass]; - } - - $response = $client->get($url, $params); - $body = $response->getBody(); + $allowLocalAccess = $this->config->getAppValue('dav', 'webcalAllowLocalAccess', 'no'); - if ($latestLocation) { - $mutations['{http://calendarserver.org/ns/}source'] = new Href($latestLocation); - } + $params = [ + 'nextcloud' => [ + 'allow_local_address' => $allowLocalAccess === 'yes', + ], + RequestOptions::HEADERS => [ + 'User-Agent' => 'Nextcloud Webcal Service', + 'Accept' => 'text/calendar, application/calendar+json, application/calendar+xml', + ], + ]; + + $user = parse_url($subscription['source'], PHP_URL_USER); + $pass = parse_url($subscription['source'], PHP_URL_PASS); + if ($user !== null && $pass !== null) { + $params[RequestOptions::AUTH] = [$user, $pass]; + } - $contentType = $response->getHeader('Content-Type'); - $contentType = explode(';', $contentType, 2)[0]; - switch ($contentType) { - case 'application/calendar+json': - try { - $jCalendar = Reader::readJson($body, Reader::OPTION_FORGIVING); - } catch (Exception $ex) { - // In case of a parsing error return null - $this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]); - return null; - } - return $jCalendar->serialize(); - - case 'application/calendar+xml': - try { - $xCalendar = Reader::readXML($body); - } catch (Exception $ex) { - // In case of a parsing error return null - $this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]); - return null; - } - return $xCalendar->serialize(); - - case 'text/calendar': - default: - try { - $vCalendar = Reader::read($body); - } catch (Exception $ex) { - // In case of a parsing error return null - $this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]); - return null; - } - return $vCalendar->serialize(); - } + try { + $client = $this->clientService->newClient(); + $response = $client->get($url, $params); } catch (LocalServerException $ex) { $this->logger->warning("Subscription $subscriptionId was not refreshed because it violates local access rules", [ 'exception' => $ex, ]); - return null; } catch (Exception $ex) { $this->logger->warning("Subscription $subscriptionId could not be refreshed due to a network error", [ 'exception' => $ex, ]); - return null; } + + $body = $response->getBody(); + + $contentType = $response->getHeader('Content-Type'); + $contentType = explode(';', $contentType, 2)[0]; + switch ($contentType) { + case 'application/calendar+json': + try { + $jCalendar = Reader::readJson($body, Reader::OPTION_FORGIVING); + } catch (Exception $ex) { + // In case of a parsing error return null + $this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]); + return null; + } + return $jCalendar->serialize(); + + case 'application/calendar+xml': + try { + $xCalendar = Reader::readXML($body); + } catch (Exception $ex) { + // In case of a parsing error return null + $this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]); + return null; + } + return $xCalendar->serialize(); + + case 'text/calendar': + default: + try { + $vCalendar = Reader::read($body); + } catch (Exception $ex) { + // In case of a parsing error return null + $this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]); + return null; + } + return $vCalendar->serialize(); + } } /** diff --git a/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php b/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php index 5ae62ea8b74f1..62bfd5f828dec 100644 --- a/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php +++ b/apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php @@ -26,7 +26,6 @@ */ namespace OCA\DAV\Tests\unit\CalDAV\WebcalCaching; -use GuzzleHttp\HandlerStack; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\WebcalCaching\RefreshWebcalService; use OCP\Http\Client\IClient; @@ -120,9 +119,7 @@ public function testRun(string $body, string $contentType, string $result): void $client->expects($this->once()) ->method('get') - ->with('https://foo.bar/bla2', $this->callback(function ($obj) { - return $obj['allow_redirects']['redirects'] === 10 && $obj['handler'] instanceof HandlerStack; - })) + ->with('https://foo.bar/bla2') ->willReturn($response); $response->expects($this->once()) @@ -188,9 +185,7 @@ public function testRunCreateCalendarNoException(string $body, string $contentTy $client->expects($this->once()) ->method('get') - ->with('https://foo.bar/bla2', $this->callback(function ($obj) { - return $obj['allow_redirects']['redirects'] === 10 && $obj['handler'] instanceof HandlerStack; - })) + ->with('https://foo.bar/bla2') ->willReturn($response); $response->expects($this->once()) @@ -212,7 +207,7 @@ public function testRunCreateCalendarNoException(string $body, string $contentTy $noInstanceException = new NoInstancesException("can't add calendar object"); $this->caldavBackend->expects($this->once()) - ->method("createCalendarObject") + ->method('createCalendarObject') ->willThrowException($noInstanceException); $this->logger->expects($this->once()) @@ -265,9 +260,7 @@ public function testRunCreateCalendarBadRequest(string $body, string $contentTyp $client->expects($this->once()) ->method('get') - ->with('https://foo.bar/bla2', $this->callback(function ($obj) { - return $obj['allow_redirects']['redirects'] === 10 && $obj['handler'] instanceof HandlerStack; - })) + ->with('https://foo.bar/bla2') ->willReturn($response); $response->expects($this->once()) @@ -289,7 +282,7 @@ public function testRunCreateCalendarBadRequest(string $body, string $contentTyp $badRequestException = new BadRequest("can't add reach calendar url"); $this->caldavBackend->expects($this->once()) - ->method("createCalendarObject") + ->method('createCalendarObject') ->willThrowException($badRequestException); $this->logger->expects($this->once()) @@ -367,7 +360,7 @@ public function testRunLocalURL(string $source): void { $this->logger->expects($this->once()) ->method('warning') - ->with("Subscription 42 was not refreshed because it violates local access rules", ['exception' => $localServerException]); + ->with('Subscription 42 was not refreshed because it violates local access rules', ['exception' => $localServerException]); $refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123'); } @@ -411,15 +404,11 @@ public function testInvalidUrl(): void { ]); $client = $this->createMock(IClient::class); - $this->clientService->expects($this->once()) - ->method('newClient') - ->with() - ->willReturn($client); + $this->clientService->expects($this->never()) + ->method('newClient'); - $this->config->expects($this->once()) - ->method('getAppValue') - ->with('dav', 'webcalAllowLocalAccess', 'no') - ->willReturn('no'); + $this->config->expects($this->never()) + ->method('getAppValue'); $client->expects($this->never()) ->method('get');