Skip to content

Commit 528d121

Browse files
nickvergessennickvergessen
authored
Merge pull request #48526 from nextcloud/backport/48510/stable27
[stable27] fix(caldav): add missing handlers
2 parents e80b4ed + 9c594cd commit 528d121

File tree

2 files changed

+71
-111
lines changed

2 files changed

+71
-111
lines changed

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

Lines changed: 61 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,14 @@
3131
namespace OCA\DAV\CalDAV\WebcalCaching;
3232

3333
use Exception;
34-
use GuzzleHttp\HandlerStack;
35-
use GuzzleHttp\Middleware;
34+
use GuzzleHttp\RequestOptions;
3635
use OCA\DAV\CalDAV\CalDavBackend;
3736
use OCP\Http\Client\IClientService;
3837
use OCP\Http\Client\LocalServerException;
3938
use OCP\IConfig;
40-
use Psr\Http\Message\RequestInterface;
41-
use Psr\Http\Message\ResponseInterface;
4239
use Psr\Log\LoggerInterface;
4340
use Sabre\DAV\Exception\BadRequest;
4441
use Sabre\DAV\PropPatch;
45-
use Sabre\DAV\Xml\Property\Href;
4642
use Sabre\VObject\Component;
4743
use Sabre\VObject\DateTimeParser;
4844
use Sabre\VObject\InvalidDataException;
@@ -76,15 +72,15 @@ public function __construct(CalDavBackend $calDavBackend, IClientService $client
7672
$this->logger = $logger;
7773
}
7874

79-
public function refreshSubscription(string $principalUri, string $uri) {
75+
public function refreshSubscription(string $principalUri, string $uri): void {
8076
$subscription = $this->getSubscription($principalUri, $uri);
8177
$mutations = [];
8278
if (!$subscription) {
8379
return;
8480
}
8581

8682
$webcalData = $this->queryWebcalFeed($subscription, $mutations);
87-
if (!$webcalData) {
83+
if ($webcalData === null) {
8884
return;
8985
}
9086

@@ -127,7 +123,7 @@ public function refreshSubscription(string $principalUri, string $uri) {
127123
$calendarData = $vObject->serialize();
128124
try {
129125
$this->calDavBackend->createCalendarObject($subscription['id'], $objectUri, $calendarData, CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION);
130-
} catch (NoInstancesException | BadRequest $ex) {
126+
} catch (NoInstancesException|BadRequest $ex) {
131127
$this->logger->error('Unable to create calendar object from subscription {subscriptionId}', ['exception' => $ex, 'subscriptionId' => $subscription['id'], 'source' => $subscription['source']]);
132128
}
133129
}
@@ -139,7 +135,7 @@ public function refreshSubscription(string $principalUri, string $uri) {
139135

140136
$this->updateSubscription($subscription, $mutations);
141137
} catch (ParseException $ex) {
142-
$this->logger->error("Subscription {subscriptionId} could not be refreshed due to a parsing error", ['exception' => $ex, 'subscriptionId' => $subscription['id']]);
138+
$this->logger->error('Subscription {subscriptionId} could not be refreshed due to a parsing error', ['exception' => $ex, 'subscriptionId' => $subscription['id']]);
143139
}
144140
}
145141

@@ -165,106 +161,81 @@ function ($sub) use ($uri) {
165161
* gets webcal feed from remote server
166162
*/
167163
private function queryWebcalFeed(array $subscription, array &$mutations): ?string {
168-
$client = $this->clientService->newClient();
169-
170-
$didBreak301Chain = false;
171-
$latestLocation = null;
172-
173-
$handlerStack = HandlerStack::create();
174-
$handlerStack->push(Middleware::mapRequest(function (RequestInterface $request) {
175-
return $request
176-
->withHeader('Accept', 'text/calendar, application/calendar+json, application/calendar+xml')
177-
->withHeader('User-Agent', 'Nextcloud Webcal Service');
178-
}));
179-
$handlerStack->push(Middleware::mapResponse(function (ResponseInterface $response) use (&$didBreak301Chain, &$latestLocation) {
180-
if (!$didBreak301Chain) {
181-
if ($response->getStatusCode() !== 301) {
182-
$didBreak301Chain = true;
183-
} else {
184-
$latestLocation = $response->getHeader('Location');
185-
}
186-
}
187-
return $response;
188-
}));
189-
190-
$allowLocalAccess = $this->config->getAppValue('dav', 'webcalAllowLocalAccess', 'no');
191164
$subscriptionId = $subscription['id'];
192165
$url = $this->cleanURL($subscription['source']);
193166
if ($url === null) {
194167
return null;
195168
}
196169

197-
try {
198-
$params = [
199-
'allow_redirects' => [
200-
'redirects' => 10
201-
],
202-
'handler' => $handlerStack,
203-
'nextcloud' => [
204-
'allow_local_address' => $allowLocalAccess === 'yes',
205-
]
206-
];
207-
208-
$user = parse_url($subscription['source'], PHP_URL_USER);
209-
$pass = parse_url($subscription['source'], PHP_URL_PASS);
210-
if ($user !== null && $pass !== null) {
211-
$params['auth'] = [$user, $pass];
212-
}
213-
214-
$response = $client->get($url, $params);
215-
$body = $response->getBody();
170+
$allowLocalAccess = $this->config->getAppValue('dav', 'webcalAllowLocalAccess', 'no');
216171

217-
if ($latestLocation) {
218-
$mutations['{http://calendarserver.org/ns/}source'] = new Href($latestLocation);
219-
}
172+
$params = [
173+
'nextcloud' => [
174+
'allow_local_address' => $allowLocalAccess === 'yes',
175+
],
176+
RequestOptions::HEADERS => [
177+
'User-Agent' => 'Nextcloud Webcal Service',
178+
'Accept' => 'text/calendar, application/calendar+json, application/calendar+xml',
179+
],
180+
];
181+
182+
$user = parse_url($subscription['source'], PHP_URL_USER);
183+
$pass = parse_url($subscription['source'], PHP_URL_PASS);
184+
if ($user !== null && $pass !== null) {
185+
$params[RequestOptions::AUTH] = [$user, $pass];
186+
}
220187

221-
$contentType = $response->getHeader('Content-Type');
222-
$contentType = explode(';', $contentType, 2)[0];
223-
switch ($contentType) {
224-
case 'application/calendar+json':
225-
try {
226-
$jCalendar = Reader::readJson($body, Reader::OPTION_FORGIVING);
227-
} catch (Exception $ex) {
228-
// In case of a parsing error return null
229-
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
230-
return null;
231-
}
232-
return $jCalendar->serialize();
233-
234-
case 'application/calendar+xml':
235-
try {
236-
$xCalendar = Reader::readXML($body);
237-
} catch (Exception $ex) {
238-
// In case of a parsing error return null
239-
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
240-
return null;
241-
}
242-
return $xCalendar->serialize();
243-
244-
case 'text/calendar':
245-
default:
246-
try {
247-
$vCalendar = Reader::read($body);
248-
} catch (Exception $ex) {
249-
// In case of a parsing error return null
250-
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
251-
return null;
252-
}
253-
return $vCalendar->serialize();
254-
}
188+
try {
189+
$client = $this->clientService->newClient();
190+
$response = $client->get($url, $params);
255191
} catch (LocalServerException $ex) {
256192
$this->logger->warning("Subscription $subscriptionId was not refreshed because it violates local access rules", [
257193
'exception' => $ex,
258194
]);
259-
260195
return null;
261196
} catch (Exception $ex) {
262197
$this->logger->warning("Subscription $subscriptionId could not be refreshed due to a network error", [
263198
'exception' => $ex,
264199
]);
265-
266200
return null;
267201
}
202+
203+
$body = $response->getBody();
204+
205+
$contentType = $response->getHeader('Content-Type');
206+
$contentType = explode(';', $contentType, 2)[0];
207+
switch ($contentType) {
208+
case 'application/calendar+json':
209+
try {
210+
$jCalendar = Reader::readJson($body, Reader::OPTION_FORGIVING);
211+
} catch (Exception $ex) {
212+
// In case of a parsing error return null
213+
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
214+
return null;
215+
}
216+
return $jCalendar->serialize();
217+
218+
case 'application/calendar+xml':
219+
try {
220+
$xCalendar = Reader::readXML($body);
221+
} catch (Exception $ex) {
222+
// In case of a parsing error return null
223+
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
224+
return null;
225+
}
226+
return $xCalendar->serialize();
227+
228+
case 'text/calendar':
229+
default:
230+
try {
231+
$vCalendar = Reader::read($body);
232+
} catch (Exception $ex) {
233+
// In case of a parsing error return null
234+
$this->logger->warning("Subscription $subscriptionId could not be parsed", ['exception' => $ex]);
235+
return null;
236+
}
237+
return $vCalendar->serialize();
238+
}
268239
}
269240

270241
/**

‎apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php‎

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
*/
2727
namespace OCA\DAV\Tests\unit\CalDAV\WebcalCaching;
2828

29-
use GuzzleHttp\HandlerStack;
3029
use OCA\DAV\CalDAV\CalDavBackend;
3130
use OCA\DAV\CalDAV\WebcalCaching\RefreshWebcalService;
3231
use OCP\Http\Client\IClient;
@@ -120,9 +119,7 @@ public function testRun(string $body, string $contentType, string $result): void
120119

121120
$client->expects($this->once())
122121
->method('get')
123-
->with('https://foo.bar/bla2', $this->callback(function ($obj) {
124-
return $obj['allow_redirects']['redirects'] === 10 && $obj['handler'] instanceof HandlerStack;
125-
}))
122+
->with('https://foo.bar/bla2')
126123
->willReturn($response);
127124

128125
$response->expects($this->once())
@@ -188,9 +185,7 @@ public function testRunCreateCalendarNoException(string $body, string $contentTy
188185

189186
$client->expects($this->once())
190187
->method('get')
191-
->with('https://foo.bar/bla2', $this->callback(function ($obj) {
192-
return $obj['allow_redirects']['redirects'] === 10 && $obj['handler'] instanceof HandlerStack;
193-
}))
188+
->with('https://foo.bar/bla2')
194189
->willReturn($response);
195190

196191
$response->expects($this->once())
@@ -212,7 +207,7 @@ public function testRunCreateCalendarNoException(string $body, string $contentTy
212207

213208
$noInstanceException = new NoInstancesException("can't add calendar object");
214209
$this->caldavBackend->expects($this->once())
215-
->method("createCalendarObject")
210+
->method('createCalendarObject')
216211
->willThrowException($noInstanceException);
217212

218213
$this->logger->expects($this->once())
@@ -265,9 +260,7 @@ public function testRunCreateCalendarBadRequest(string $body, string $contentTyp
265260

266261
$client->expects($this->once())
267262
->method('get')
268-
->with('https://foo.bar/bla2', $this->callback(function ($obj) {
269-
return $obj['allow_redirects']['redirects'] === 10 && $obj['handler'] instanceof HandlerStack;
270-
}))
263+
->with('https://foo.bar/bla2')
271264
->willReturn($response);
272265

273266
$response->expects($this->once())
@@ -289,7 +282,7 @@ public function testRunCreateCalendarBadRequest(string $body, string $contentTyp
289282

290283
$badRequestException = new BadRequest("can't add reach calendar url");
291284
$this->caldavBackend->expects($this->once())
292-
->method("createCalendarObject")
285+
->method('createCalendarObject')
293286
->willThrowException($badRequestException);
294287

295288
$this->logger->expects($this->once())
@@ -367,7 +360,7 @@ public function testRunLocalURL(string $source): void {
367360

368361
$this->logger->expects($this->once())
369362
->method('warning')
370-
->with("Subscription 42 was not refreshed because it violates local access rules", ['exception' => $localServerException]);
363+
->with('Subscription 42 was not refreshed because it violates local access rules', ['exception' => $localServerException]);
371364

372365
$refreshWebcalService->refreshSubscription('principals/users/testuser', 'sub123');
373366
}
@@ -411,15 +404,11 @@ public function testInvalidUrl(): void {
411404
]);
412405

413406
$client = $this->createMock(IClient::class);
414-
$this->clientService->expects($this->once())
415-
->method('newClient')
416-
->with()
417-
->willReturn($client);
407+
$this->clientService->expects($this->never())
408+
->method('newClient');
418409

419-
$this->config->expects($this->once())
420-
->method('getAppValue')
421-
->with('dav', 'webcalAllowLocalAccess', 'no')
422-
->willReturn('no');
410+
$this->config->expects($this->never())
411+
->method('getAppValue');
423412

424413
$client->expects($this->never())
425414
->method('get');

0 commit comments

Comments
 (0)