Skip to content

Commit 004d7da

Browse files
miaulalalamiaulalala
authored
Merge pull request #48508 from nextcloud/fix/webcal-subscription-jobs-middleware
fix(caldav): add missing handlers
2 parents 2d004c3 + d30c066 commit 004d7da

File tree

3 files changed

+66
-108
lines changed

3 files changed

+66
-108
lines changed

‎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
@@ -58,7 +58,7 @@ public function refreshSubscription(string $principalUri, string $uri) {
5858
}
5959

6060

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

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

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88
namespace OCA\DAV\Tests\unit\CalDAV\WebcalCaching;
99

10-
use GuzzleHttp\HandlerStack;
1110
use OCA\DAV\CalDAV\WebcalCaching\Connection;
1211
use OCP\Http\Client\IClient;
1312
use OCP\Http\Client\IClientService;
@@ -48,7 +47,6 @@ public function testLocalUrl($source): void {
4847
'source' => $source,
4948
'lastmodified' => 0,
5049
];
51-
$mutation = [];
5250

5351
$client = $this->createMock(IClient::class);
5452
$this->clientService->expects(self::once())
@@ -69,7 +67,7 @@ public function testLocalUrl($source): void {
6967
->method('warning')
7068
->with('Subscription 42 was not refreshed because it violates local access rules', ['exception' => $localServerException]);
7169

72-
$this->connection->queryWebcalFeed($subscription, $mutation);
70+
$this->connection->queryWebcalFeed($subscription);
7371
}
7472

7573
public function testInvalidUrl(): void {
@@ -83,22 +81,14 @@ public function testInvalidUrl(): void {
8381
'source' => '!@#$',
8482
'lastmodified' => 0,
8583
];
86-
$mutation = [];
8784

8885
$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-
86+
$this->config->expects(self::never())
87+
->method('getValueString');
9888
$client->expects(self::never())
9989
->method('get');
10090

101-
$this->connection->queryWebcalFeed($subscription, $mutation);
91+
$this->connection->queryWebcalFeed($subscription);
10292

10393
}
10494

@@ -120,7 +110,6 @@ public function testConnection(string $url, string $result, string $contentType)
120110
'source' => $url,
121111
'lastmodified' => 0,
122112
];
123-
$mutation = [];
124113

125114
$this->clientService->expects($this->once())
126115
->method('newClient')
@@ -134,9 +123,7 @@ public function testConnection(string $url, string $result, string $contentType)
134123

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

142129
$response->expects($this->once())
@@ -148,9 +135,9 @@ public function testConnection(string $url, string $result, string $contentType)
148135
->with('Content-Type')
149136
->willReturn($contentType);
150137

151-
$this->connection->queryWebcalFeed($subscription, $mutation);
152-
138+
$this->connection->queryWebcalFeed($subscription);
153139
}
140+
154141
public static function runLocalURLDataProvider(): array {
155142
return [
156143
['localhost/foo.bar'],

0 commit comments

Comments
 (0)