Skip to content
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

Jump to bottom

[stable29] fix(caldav): add missing handlers #48509

Merged
merged 1 commit into from
Oct 2, 2024
Merged

[stable29] fix(caldav): add missing handlers #48509

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
157 changes: 64 additions & 93 deletions apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 OCP\IAppConfig;
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;
Expand All @@ -59,7 +55,7 @@ class RefreshWebcalService {

private IClientService $clientService;

private IConfig $config;
private IAppConfig $config;

/** @var LoggerInterface */
private LoggerInterface $logger;
Expand All @@ -69,22 +65,22 @@ class RefreshWebcalService {
public const STRIP_ATTACHMENTS = '{http://calendarserver.org/ns/}subscribed-strip-attachments';
public const STRIP_TODOS = '{http://calendarserver.org/ns/}subscribed-strip-todos';

public function __construct(CalDavBackend $calDavBackend, IClientService $clientService, IConfig $config, LoggerInterface $logger) {
public function __construct(CalDavBackend $calDavBackend, IClientService $clientService, IAppConfig $config, LoggerInterface $logger) {
$this->calDavBackend = $calDavBackend;
$this->clientService = $clientService;
$this->config = $config;
$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) {
return;
}

$webcalData = $this->queryWebcalFeed($subscription, $mutations);
if (!$webcalData) {
if ($webcalData === null) {
return;
}

Expand Down Expand Up @@ -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']]);
}
}
Expand All @@ -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']]);
}
}

Expand All @@ -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];
}
$allowLocalAccess = $this->config->getValueString('dav', 'webcalAllowLocalAccess', 'no');

$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];
}

try {
$client = $this->clientService->newClient();
$response = $client->get($url, $params);
$body = $response->getBody();

if ($latestLocation) {
$mutations['{http://calendarserver.org/ns/}source'] = new Href($latestLocation);
}

$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();
}
} 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();
}
}

/**
Expand Down
45 changes: 17 additions & 28 deletions apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,13 @@
*/
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;
use OCP\Http\Client\IClientService;
use OCP\Http\Client\IResponse;
use OCP\Http\Client\LocalServerException;
use OCP\IConfig;
use OCP\IAppConfig;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Sabre\DAV\Exception\BadRequest;
Expand All @@ -50,7 +49,7 @@ class RefreshWebcalServiceTest extends TestCase {
/** @var IClientService | MockObject */
private $clientService;

/** @var IConfig | MockObject */
/** @var IAppConfig | MockObject */
private $config;

/** @var LoggerInterface | MockObject */
Expand All @@ -61,7 +60,7 @@ protected function setUp(): void {

$this->caldavBackend = $this->createMock(CalDavBackend::class);
$this->clientService = $this->createMock(IClientService::class);
$this->config = $this->createMock(IConfig::class);
$this->config = $this->createMock(IAppConfig::class);
$this->logger = $this->createMock(LoggerInterface::class);
}

Expand Down Expand Up @@ -114,15 +113,13 @@ public function testRun(string $body, string $contentType, string $result): void
->willReturn($client);

$this->config->expects($this->once())
->method('getAppValue')
->method('getValueString')
->with('dav', 'webcalAllowLocalAccess', 'no')
->willReturn('no');

$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())
Expand Down Expand Up @@ -182,15 +179,13 @@ public function testRunCreateCalendarNoException(string $body, string $contentTy
->willReturn($client);

$this->config->expects($this->once())
->method('getAppValue')
->method('getValueString')
->with('dav', 'webcalAllowLocalAccess', 'no')
->willReturn('no');

$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())
Expand All @@ -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())
Expand Down Expand Up @@ -259,15 +254,13 @@ public function testRunCreateCalendarBadRequest(string $body, string $contentTyp
->willReturn($client);

$this->config->expects($this->once())
->method('getAppValue')
->method('getValueString')
->with('dav', 'webcalAllowLocalAccess', 'no')
->willReturn('no');

$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())
Expand All @@ -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())
Expand Down Expand Up @@ -355,7 +348,7 @@ public function testRunLocalURL(string $source): void {
->willReturn($client);

$this->config->expects($this->once())
->method('getAppValue')
->method('getValueString')
->with('dav', 'webcalAllowLocalAccess', 'no')
->willReturn('no');

Expand All @@ -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');
}
Expand Down Expand Up @@ -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('getValueString');

$client->expects($this->never())
->method('get');
Expand Down
Loading