Skip to content

Commit 836a0c2

Browse files
committed
fix(caldav): add missing handlers
Signed-off-by: Anna Larch <[email protected]>
1 parent 1c1f488 commit 836a0c2

3 files changed

Lines changed: 66 additions & 107 deletions

File tree

apps/dav/lib/CalDAV/WebcalCaching/Connection.php

Lines changed: 58 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,11 @@
99
namespace OCA\DAV\CalDAV\WebcalCaching;
1010

1111
use Exception;
12-
use GuzzleHttp\HandlerStack;
13-
use GuzzleHttp\Middleware;
12+
use GuzzleHttp\RequestOptions;
1413
use OCP\Http\Client\IClientService;
1514
use OCP\Http\Client\LocalServerException;
1615
use OCP\IAppConfig;
17-
use Psr\Http\Message\RequestInterface;
18-
use Psr\Http\Message\ResponseInterface;
1916
use Psr\Log\LoggerInterface;
20-
use Sabre\DAV\Xml\Property\Href;
2117
use Sabre\VObject\Reader;
2218

2319
class Connection {
@@ -31,107 +27,82 @@ public function __construct(
3127
/**
3228
* gets webcal feed from remote server
3329
*/
34-
public function queryWebcalFeed(array $subscription, array &$mutations): ?string {
35-
$client = $this->clientService->newClient();
36-
37-
$didBreak301Chain = false;
38-
$latestLocation = null;
39-
40-
$handlerStack = HandlerStack::create();
41-
$handlerStack->push(Middleware::mapRequest(function (RequestInterface $request) {
42-
return $request
43-
->withHeader('Accept', 'text/calendar, application/calendar+json, application/calendar+xml')
44-
->withHeader('User-Agent', 'Nextcloud Webcal Service');
45-
}));
46-
$handlerStack->push(Middleware::mapResponse(function (ResponseInterface $response) use (&$didBreak301Chain, &$latestLocation) {
47-
if (!$didBreak301Chain) {
48-
if ($response->getStatusCode() !== 301) {
49-
$didBreak301Chain = true;
50-
} else {
51-
$latestLocation = $response->getHeader('Location');
52-
}
53-
}
54-
return $response;
55-
}));
56-
57-
$allowLocalAccess = $this->config->getValueString('dav', 'webcalAllowLocalAccess', 'no');
30+
public function queryWebcalFeed(array $subscription): ?string {
5831
$subscriptionId = $subscription['id'];
5932
$url = $this->cleanURL($subscription['source']);
6033
if ($url === null) {
6134
return null;
6235
}
6336

64-
try {
65-
$params = [
66-
'allow_redirects' => [
67-
'redirects' => 10
68-
],
69-
'handler' => $handlerStack,
70-
'nextcloud' => [
71-
'allow_local_address' => $allowLocalAccess === 'yes',
72-
]
73-
];
74-
75-
$user = parse_url($subscription['source'], PHP_URL_USER);
76-
$pass = parse_url($subscription['source'], PHP_URL_PASS);
77-
if ($user !== null && $pass !== null) {
78-
$params['auth'] = [$user, $pass];
79-
}
37+
$allowLocalAccess = $this->config->getValueString('dav', 'webcalAllowLocalAccess', 'no');
38+
39+
$params = [
40+
'nextcloud' => [
41+
'allow_local_address' => $allowLocalAccess === 'yes',
42+
],
43+
RequestOptions::HEADERS => [
44+
'User-Agent' => 'Nextcloud Webcal Service',
45+
'Accept' => 'text/calendar, application/calendar+json, application/calendar+xml',
46+
],
47+
];
48+
49+
$user = parse_url($subscription['source'], PHP_URL_USER);
50+
$pass = parse_url($subscription['source'], PHP_URL_PASS);
51+
if ($user !== null && $pass !== null) {
52+
$params[RequestOptions::AUTH] = [$user, $pass];
53+
}
8054

55+
try {
56+
$client = $this->clientService->newClient();
8157
$response = $client->get($url, $params);
82-
$body = $response->getBody();
83-
84-
if ($latestLocation !== null) {
85-
$mutations['{http://calendarserver.org/ns/}source'] = new Href($latestLocation);
86-
}
87-
88-
$contentType = $response->getHeader('Content-Type');
89-
$contentType = explode(';', $contentType, 2)[0];
90-
switch ($contentType) {
91-
case 'application/calendar+json':
92-
try {
93-
$jCalendar = Reader::readJson($body, Reader::OPTION_FORGIVING);
94-
} catch (Exception $ex) {
95-
// In case of a parsing error return null
96-
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
97-
return null;
98-
}
99-
return $jCalendar->serialize();
100-
101-
case 'application/calendar+xml':
102-
try {
103-
$xCalendar = Reader::readXML($body);
104-
} catch (Exception $ex) {
105-
// In case of a parsing error return null
106-
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
107-
return null;
108-
}
109-
return $xCalendar->serialize();
110-
111-
case 'text/calendar':
112-
default:
113-
try {
114-
$vCalendar = Reader::read($body);
115-
} catch (Exception $ex) {
116-
// In case of a parsing error return null
117-
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
118-
return null;
119-
}
120-
return $vCalendar->serialize();
121-
}
12258
} catch (LocalServerException $ex) {
12359
$this->logger->warning("Subscription $subscriptionId was not refreshed because it violates local access rules", [
12460
'exception' => $ex,
12561
]);
126-
12762
return null;
12863
} catch (Exception $ex) {
12964
$this->logger->warning("Subscription $subscriptionId could not be refreshed due to a network error", [
13065
'exception' => $ex,
13166
]);
132-
13367
return null;
13468
}
69+
70+
$body = $response->getBody();
71+
72+
$contentType = $response->getHeader('Content-Type');
73+
$contentType = explode(';', $contentType, 2)[0];
74+
switch ($contentType) {
75+
case 'application/calendar+json':
76+
try {
77+
$jCalendar = Reader::readJson($body, Reader::OPTION_FORGIVING);
78+
} catch (Exception $ex) {
79+
// In case of a parsing error return null
80+
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
81+
return null;
82+
}
83+
return $jCalendar->serialize();
84+
85+
case 'application/calendar+xml':
86+
try {
87+
$xCalendar = Reader::readXML($body);
88+
} catch (Exception $ex) {
89+
// In case of a parsing error return null
90+
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
91+
return null;
92+
}
93+
return $xCalendar->serialize();
94+
95+
case 'text/calendar':
96+
default:
97+
try {
98+
$vCalendar = Reader::read($body);
99+
} catch (Exception $ex) {
100+
// In case of a parsing error return null
101+
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
102+
return null;
103+
}
104+
return $vCalendar->serialize();
105+
}
135106
}
136107

137108
/**

apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public function refreshSubscription(string $principalUri, string $uri) {
5757
}
5858

5959

60-
$webcalData = $this->connection->queryWebcalFeed($subscription, $mutations);
60+
$webcalData = $this->connection->queryWebcalFeed($subscription);
6161
if (!$webcalData) {
6262
return;
6363
}

apps/dav/tests/unit/CalDAV/WebcalCaching/ConnectionTest.php

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ public function testLocalUrl($source): void {
4848
'source' => $source,
4949
'lastmodified' => 0,
5050
];
51-
$mutation = [];
5251

5352
$client = $this->createMock(IClient::class);
5453
$this->clientService->expects(self::once())
@@ -69,7 +68,7 @@ public function testLocalUrl($source): void {
6968
->method('warning')
7069
->with('Subscription 42 was not refreshed because it violates local access rules', ['exception' => $localServerException]);
7170

72-
$this->connection->queryWebcalFeed($subscription, $mutation);
71+
$this->connection->queryWebcalFeed($subscription);
7372
}
7473

7574
public function testInvalidUrl(): void {
@@ -83,22 +82,14 @@ public function testInvalidUrl(): void {
8382
'source' => '!@#$',
8483
'lastmodified' => 0,
8584
];
86-
$mutation = [];
8785

8886
$client = $this->createMock(IClient::class);
89-
$this->clientService->expects(self::once())
90-
->method('newClient')
91-
->with()
92-
->willReturn($client);
93-
$this->config->expects(self::once())
94-
->method('getValueString')
95-
->with('dav', 'webcalAllowLocalAccess', 'no')
96-
->willReturn('no');
97-
87+
$this->config->expects(self::never())
88+
->method('getValueString');
9889
$client->expects(self::never())
9990
->method('get');
10091

101-
$this->connection->queryWebcalFeed($subscription, $mutation);
92+
$this->connection->queryWebcalFeed($subscription);
10293

10394
}
10495

@@ -120,7 +111,6 @@ public function testConnection(string $url, string $result, string $contentType)
120111
'source' => $url,
121112
'lastmodified' => 0,
122113
];
123-
$mutation = [];
124114

125115
$this->clientService->expects($this->once())
126116
->method('newClient')
@@ -134,9 +124,7 @@ public function testConnection(string $url, string $result, string $contentType)
134124

135125
$client->expects($this->once())
136126
->method('get')
137-
->with('https://foo.bar/bla2', $this->callback(function ($obj) {
138-
return $obj['allow_redirects']['redirects'] === 10 && $obj['handler'] instanceof HandlerStack;
139-
}))
127+
->with('https://foo.bar/bla2')
140128
->willReturn($response);
141129

142130
$response->expects($this->once())
@@ -148,9 +136,9 @@ public function testConnection(string $url, string $result, string $contentType)
148136
->with('Content-Type')
149137
->willReturn($contentType);
150138

151-
$this->connection->queryWebcalFeed($subscription, $mutation);
152-
139+
$this->connection->queryWebcalFeed($subscription);
153140
}
141+
154142
public static function runLocalURLDataProvider(): array {
155143
return [
156144
['localhost/foo.bar'],

0 commit comments

Comments
 (0)