Skip to content

Commit 26df2fe

Browse files
artongebackportbot[bot]
artonge
authored and
backportbot[bot]
committed
fix(dav): Always respond custom error page on exceptions
Signed-off-by: Louis Chemineau <louis@chmn.me> [skip ci]
1 parent 86901b3 commit 26df2fe

File tree

10 files changed

+112
-73
lines changed

10 files changed

+112
-73
lines changed

‎apps/dav/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@
257257
'OCA\\DAV\\Events\\SubscriptionUpdatedEvent' => $baseDir . '/../lib/Events/SubscriptionUpdatedEvent.php',
258258
'OCA\\DAV\\Exception\\ServerMaintenanceMode' => $baseDir . '/../lib/Exception/ServerMaintenanceMode.php',
259259
'OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException' => $baseDir . '/../lib/Exception/UnsupportedLimitOnInitialSyncException.php',
260-
'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => $baseDir . '/../lib/Files/BrowserErrorPagePlugin.php',
260+
'OCA\\DAV\\Files\\ErrorPagePlugin' => $baseDir . '/../lib/Files/ErrorPagePlugin.php',
261261
'OCA\\DAV\\Files\\FileSearchBackend' => $baseDir . '/../lib/Files/FileSearchBackend.php',
262262
'OCA\\DAV\\Files\\FilesHome' => $baseDir . '/../lib/Files/FilesHome.php',
263263
'OCA\\DAV\\Files\\LazySearchBackend' => $baseDir . '/../lib/Files/LazySearchBackend.php',

‎apps/dav/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ class ComposerStaticInitDAV
272272
'OCA\\DAV\\Events\\SubscriptionUpdatedEvent' => __DIR__ . '/..' . '/../lib/Events/SubscriptionUpdatedEvent.php',
273273
'OCA\\DAV\\Exception\\ServerMaintenanceMode' => __DIR__ . '/..' . '/../lib/Exception/ServerMaintenanceMode.php',
274274
'OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException' => __DIR__ . '/..' . '/../lib/Exception/UnsupportedLimitOnInitialSyncException.php',
275-
'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/BrowserErrorPagePlugin.php',
275+
'OCA\\DAV\\Files\\ErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/ErrorPagePlugin.php',
276276
'OCA\\DAV\\Files\\FileSearchBackend' => __DIR__ . '/..' . '/../lib/Files/FileSearchBackend.php',
277277
'OCA\\DAV\\Files\\FilesHome' => __DIR__ . '/..' . '/../lib/Files/FilesHome.php',
278278
'OCA\\DAV\\Files\\LazySearchBackend' => __DIR__ . '/..' . '/../lib/Files/LazySearchBackend.php',

‎apps/dav/lib/Connector/Sabre/ServerFactory.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333

3434
use OCA\DAV\AppInfo\PluginManager;
3535
use OCA\DAV\DAV\ViewOnlyPlugin;
36-
use OCA\DAV\Files\BrowserErrorPagePlugin;
36+
use OCA\DAV\Files\ErrorPagePlugin;
3737
use OCP\EventDispatcher\IEventDispatcher;
3838
use OCP\Files\Folder;
3939
use OCP\Files\Mount\IMountManager;
@@ -120,9 +120,7 @@ public function createServer(string $baseUri,
120120
$server->addPlugin(new \OCA\DAV\Connector\Sabre\FakeLockerPlugin());
121121
}
122122

123-
if (BrowserErrorPagePlugin::isBrowserRequest($this->request)) {
124-
$server->addPlugin(new BrowserErrorPagePlugin());
125-
}
123+
$server->addPlugin(new ErrorPagePlugin($this->request, $this->config));
126124

127125
// wait with registering these until auth is handled and the filesystem is setup
128126
$server->on('beforeMethod:*', function () use ($server, $objectTree, $viewCallBack) {

apps/dav/lib/Files/BrowserErrorPagePlugin.php renamed to apps/dav/lib/Files/ErrorPagePlugin.php

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,22 @@
2424
*/
2525
namespace OCA\DAV\Files;
2626

27-
use OC\AppFramework\Http\Request;
2827
use OC_Template;
2928
use OCP\AppFramework\Http\ContentSecurityPolicy;
29+
use OCP\IConfig;
3030
use OCP\IRequest;
3131
use Sabre\DAV\Exception;
3232
use Sabre\DAV\Server;
3333
use Sabre\DAV\ServerPlugin;
3434

35-
class BrowserErrorPagePlugin extends ServerPlugin {
36-
/** @var Server */
37-
private $server;
35+
class ErrorPagePlugin extends ServerPlugin {
36+
private ?Server $server = null;
37+
38+
public function __construct(
39+
private IRequest $request,
40+
private IConfig $config,
41+
) {
42+
}
3843

3944
/**
4045
* This initializes the plugin.
@@ -43,35 +48,12 @@ class BrowserErrorPagePlugin extends ServerPlugin {
4348
* addPlugin is called.
4449
*
4550
* This method should set up the required event subscriptions.
46-
*
47-
* @param Server $server
48-
* @return void
4951
*/
50-
public function initialize(Server $server) {
52+
public function initialize(Server $server): void {
5153
$this->server = $server;
5254
$server->on('exception', [$this, 'logException'], 1000);
5355
}
5456

55-
/**
56-
* @param IRequest $request
57-
* @return bool
58-
*/
59-
public static function isBrowserRequest(IRequest $request) {
60-
if ($request->getMethod() !== 'GET') {
61-
return false;
62-
}
63-
return $request->isUserAgent([
64-
Request::USER_AGENT_IE,
65-
Request::USER_AGENT_MS_EDGE,
66-
Request::USER_AGENT_CHROME,
67-
Request::USER_AGENT_FIREFOX,
68-
Request::USER_AGENT_SAFARI,
69-
]);
70-
}
71-
72-
/**
73-
* @param \Throwable $ex
74-
*/
7557
public function logException(\Throwable $ex): void {
7658
if ($ex instanceof Exception) {
7759
$httpCode = $ex->getHTTPCode();
@@ -82,7 +64,7 @@ public function logException(\Throwable $ex): void {
8264
}
8365
$this->server->httpResponse->addHeaders($headers);
8466
$this->server->httpResponse->setStatus($httpCode);
85-
$body = $this->generateBody($httpCode);
67+
$body = $this->generateBody($ex, $httpCode);
8668
$this->server->httpResponse->setBody($body);
8769
$csp = new ContentSecurityPolicy();
8870
$this->server->httpResponse->addHeader('Content-Security-Policy', $csp->buildPolicy());
@@ -93,18 +75,32 @@ public function logException(\Throwable $ex): void {
9375
* @codeCoverageIgnore
9476
* @return bool|string
9577
*/
96-
public function generateBody(int $httpCode) {
97-
$request = \OC::$server->getRequest();
98-
99-
$templateName = 'exception';
100-
if ($httpCode === 403 || $httpCode === 404) {
101-
$templateName = (string)$httpCode;
78+
public function generateBody(\Throwable $ex, int $httpCode): mixed {
79+
if ($this->acceptHtml()) {
80+
$templateName = 'exception';
81+
$renderAs = 'guest';
82+
if ($httpCode === 403 || $httpCode === 404) {
83+
$templateName = (string)$httpCode;
84+
}
85+
} else {
86+
$templateName = 'xml_exception';
87+
$renderAs = null;
88+
$this->server->httpResponse->setHeader('Content-Type', 'application/xml; charset=utf-8');
10289
}
10390

104-
$content = new OC_Template('core', $templateName, 'guest');
91+
$debug = $this->config->getSystemValueBool('debug', false);
92+
93+
$content = new OC_Template('core', $templateName, $renderAs);
10594
$content->assign('title', $this->server->httpResponse->getStatusText());
106-
$content->assign('remoteAddr', $request->getRemoteAddress());
107-
$content->assign('requestID', $request->getId());
95+
$content->assign('remoteAddr', $this->request->getRemoteAddress());
96+
$content->assign('requestID', $this->request->getId());
97+
$content->assign('debugMode', $debug);
98+
$content->assign('errorClass', get_class($ex));
99+
$content->assign('errorMsg', $ex->getMessage());
100+
$content->assign('errorCode', $ex->getCode());
101+
$content->assign('file', $ex->getFile());
102+
$content->assign('line', $ex->getLine());
103+
$content->assign('exception', $ex);
108104
return $content->fetchPage();
109105
}
110106

@@ -115,4 +111,14 @@ public function sendResponse() {
115111
$this->server->sapi->sendResponse($this->server->httpResponse);
116112
exit();
117113
}
114+
115+
private function acceptHtml(): bool {
116+
foreach (explode(',', $this->request->getHeader('Accept')) as $part) {
117+
$subparts = explode(';', $part);
118+
if (str_ends_with($subparts[0], '/html')) {
119+
return true;
120+
}
121+
}
122+
return false;
123+
}
118124
}

‎apps/dav/lib/Server.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
use OCA\DAV\DAV\ViewOnlyPlugin;
7272
use OCA\DAV\Events\SabrePluginAddEvent;
7373
use OCA\DAV\Events\SabrePluginAuthInitEvent;
74-
use OCA\DAV\Files\BrowserErrorPagePlugin;
74+
use OCA\DAV\Files\ErrorPagePlugin;
7575
use OCA\DAV\Files\LazySearchBackend;
7676
use OCA\DAV\Profiler\ProfilerPlugin;
7777
use OCA\DAV\Provisioning\Apple\AppleProvisioningPlugin;
@@ -248,9 +248,7 @@ public function __construct(IRequest $request, string $baseUri) {
248248
$this->server->addPlugin(new FakeLockerPlugin());
249249
}
250250

251-
if (BrowserErrorPagePlugin::isBrowserRequest($request)) {
252-
$this->server->addPlugin(new BrowserErrorPagePlugin());
253-
}
251+
$this->server->addPlugin(new ErrorPagePlugin($this->request, \OC::$server->getConfig()));
254252

255253
$lazySearchBackend = new LazySearchBackend();
256254
$this->server->addPlugin(new SearchPlugin($lazySearchBackend));

‎apps/dav/tests/travis/caldavtest/tests/CalDAV/sync-report.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2712,7 +2712,7 @@
27122712
<callback>prepostcondition</callback>
27132713
<arg>
27142714
<name>error</name>
2715-
<value>{DAV:}valid-sync-token</value>
2715+
<value>{http://sabredav.org/ns}exception</value>
27162716
</arg>
27172717
<arg>
27182718
<name>ignoreextras</name>

apps/dav/tests/unit/DAV/BrowserErrorPagePluginTest.php renamed to apps/dav/tests/unit/DAV/ErrorPagePluginTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,20 @@
2323
*/
2424
namespace OCA\DAV\Tests\unit\DAV;
2525

26-
use OCA\DAV\Files\BrowserErrorPagePlugin;
26+
use OCA\DAV\Files\ErrorPagePlugin;
2727
use Sabre\DAV\Exception\NotFound;
2828
use Sabre\HTTP\Response;
2929

30-
class BrowserErrorPagePluginTest extends \Test\TestCase {
30+
class ErrorPagePluginTest extends \Test\TestCase {
3131

3232
/**
3333
* @dataProvider providesExceptions
3434
* @param $expectedCode
3535
* @param $exception
3636
*/
3737
public function test($expectedCode, $exception): void {
38-
/** @var BrowserErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */
39-
$plugin = $this->getMockBuilder(BrowserErrorPagePlugin::class)->setMethods(['sendResponse', 'generateBody'])->getMock();
38+
/** @var ErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */
39+
$plugin = $this->getMockBuilder(ErrorPagePlugin::class)->disableOriginalConstructor()->setMethods(['sendResponse', 'generateBody'])->getMock();
4040
$plugin->expects($this->once())->method('generateBody')->willReturn(':boom:');
4141
$plugin->expects($this->once())->method('sendResponse');
4242
/** @var \Sabre\DAV\Server | \PHPUnit\Framework\MockObject\MockObject $server */

‎build/integration/features/caldav.feature‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,15 @@ Feature: caldav
33
Given user "user0" exists
44
When "admin" requests calendar "user0/MyCalendar" on the endpoint "/remote.php/dav/calendars/"
55
Then The CalDAV HTTP status code should be "404"
6-
And The exception is "Sabre\DAV\Exception\NotFound"
7-
And The error message is "Node with name 'MyCalendar' could not be found"
6+
And The exception is "Internal Server Error"
87

98
Scenario: Accessing a not shared calendar of another user
109
Given user "user0" exists
1110
Given "admin" creates a calendar named "MyCalendar"
1211
Given The CalDAV HTTP status code should be "201"
1312
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/"
1413
Then The CalDAV HTTP status code should be "404"
15-
And The exception is "Sabre\DAV\Exception\NotFound"
16-
And The error message is "Calendar with name 'MyCalendar' could not be found"
14+
And The exception is "Internal Server Error"
1715

1816
Scenario: Accessing a not shared calendar of another user via the legacy endpoint
1917
Given user "user0" exists
@@ -28,8 +26,7 @@ Feature: caldav
2826
Given user "user0" exists
2927
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/"
3028
Then The CalDAV HTTP status code should be "404"
31-
And The exception is "Sabre\DAV\Exception\NotFound"
32-
And The error message is "Node with name 'MyCalendar' could not be found"
29+
And The exception is "Internal Server Error"
3330

3431
Scenario: Accessing a not existing calendar of another user via the legacy endpoint
3532
Given user "user0" exists
@@ -42,8 +39,7 @@ Feature: caldav
4239
Given user "user0" exists
4340
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/"
4441
Then The CalDAV HTTP status code should be "404"
45-
And The exception is "Sabre\DAV\Exception\NotFound"
46-
And The error message is "Node with name 'MyCalendar' could not be found"
42+
And The exception is "Internal Server Error"
4743

4844
Scenario: Creating a new calendar
4945
When "admin" creates a calendar named "MyCalendar"
@@ -64,8 +60,7 @@ Feature: caldav
6460
Given user "user0" exists
6561
When "user0" sends a create calendar request to "admin/MyCalendar2" on the endpoint "/remote.php/dav/calendars/"
6662
Then The CalDAV HTTP status code should be "404"
67-
And The exception is "Sabre\DAV\Exception\NotFound"
68-
And The error message is "Node with name 'admin' could not be found"
63+
And The exception is "Internal Server Error"
6964

7065
Scenario: Create calendar request for existing calendar of another user
7166
Given user "user0" exists

‎build/integration/features/carddav.feature‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,13 @@ Feature: carddav
22
Scenario: Accessing a not existing addressbook of another user
33
Given user "user0" exists
44
When "admin" requests addressbook "user0/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/dav/addressbooks/users/"
5-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
6-
And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found"
5+
And The CardDAV exception is "Internal Server Error"
76

87
Scenario: Accessing a not shared addressbook of another user
98
Given user "user0" exists
109
Given "admin" creates an addressbook named "MyAddressbook" with statuscode "201"
1110
When "user0" requests addressbook "admin/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/dav/addressbooks/users/"
12-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
13-
And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found"
11+
And The CardDAV exception is "Internal Server Error"
1412

1513
Scenario: Accessing a not existing addressbook of another user via legacy endpoint
1614
Given user "user0" exists
@@ -28,8 +26,7 @@ Feature: carddav
2826
Scenario: Accessing a not existing addressbook of myself
2927
Given user "user0" exists
3028
When "user0" requests addressbook "admin/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/dav/addressbooks/users/"
31-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
32-
And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found"
29+
And The CardDAV exception is "Internal Server Error"
3330

3431
Scenario: Creating a new addressbook
3532
When "admin" creates an addressbook named "MyAddressbook" with statuscode "201"
@@ -67,13 +64,11 @@ Feature: carddav
6764
Given user "user0" exists
6865
When "user0" sends a create addressbook request to "admin/MyAddressbook2" on the endpoint "/remote.php/dav/addressbooks/"
6966
Then The CardDAV HTTP status code should be "404"
70-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
71-
And The CardDAV error message is "File not found: admin in 'addressbooks'"
67+
And The CardDAV exception is "Internal Server Error"
7268

7369
Scenario: Create addressbook request for existing addressbook of another user
7470
Given user "user0" exists
7571
When "admin" creates an addressbook named "MyAddressbook2" with statuscode "201"
7672
When "user0" sends a create addressbook request to "admin/MyAddressbook2" on the endpoint "/remote.php/dav/addressbooks/"
7773
Then The CardDAV HTTP status code should be "404"
78-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
79-
And The CardDAV error message is "File not found: admin in 'addressbooks'"
74+
And The CardDAV exception is "Internal Server Error"

‎core/templates/xml_exception.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
/**
3+
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
4+
* SPDX-FileCopyrightText: 2012-2015 ownCloud, Inc.
5+
* SPDX-License-Identifier: AGPL-3.0-only
6+
*/
7+
8+
function print_exception(Throwable $e, \OCP\IL10N $l): void {
9+
p($e->getTraceAsString());
10+
11+
if ($e->getPrevious() !== null) {
12+
print_unescaped('<s:previous-exception>');
13+
print_exception($e->getPrevious(), $l);
14+
print_unescaped('</s:previous-exception>');
15+
}
16+
}
17+
18+
?>
19+
<?xml version="1.0" encoding="utf-8"?>
20+
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
21+
<s:exception><?php p($l->t('Internal Server Error')) ?></s:exception>
22+
<s:message>
23+
<?php p($l->t('The server was unable to complete your request.')) ?>
24+
<?php p($l->t('If this happens again, please send the technical details below to the server administrator.')) ?>
25+
<?php p($l->t('More details can be found in the server log.')) ?>
26+
<?php if (isset($_['serverLogsDocumentation']) && $_['serverLogsDocumentation'] !== ''): ?>
27+
<?php p($l->t('For more details see the documentation ↗.'))?>: <?php print_unescaped($_['serverLogsDocumentation']) ?>
28+
<?php endif; ?>
29+
</s:message>
30+
31+
<s:technical-details>
32+
<s:remote-address><?php p($_['remoteAddr']) ?></s:remote-address>
33+
<s:request-id><?php p($_['requestID']) ?></s:request-id>
34+
35+
<?php if (isset($_['debugMode']) && $_['debugMode'] === true): ?>
36+
<s:type><?php p($_['errorClass']) ?></s:type>
37+
<s:code><?php p($_['errorCode']) ?></s:code>
38+
<s:message><?php p($_['errorMsg']) ?></s:message>
39+
<s:file><?php p($_['file']) ?></s:file>
40+
<s:line><?php p($_['line']) ?></s:line>
41+
42+
<s:stacktrace>
43+
<?php print_exception($_['exception'], $l); ?>
44+
</s:stacktrace>
45+
<?php endif; ?>
46+
</s:technical-details>
47+
</d:error>

0 commit comments

Comments
 (0)