Skip to content

Commit 008d0ff

Browse files
artongeartonge
committed
fix(dav): Always respond custom error page on exceptions
Signed-off-by: Louis Chemineau <louis@chmn.me>
1 parent 332b3ef commit 008d0ff

File tree

10 files changed

+132
-83
lines changed

10 files changed

+132
-83
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@
272272
'OCA\\DAV\\Events\\SubscriptionUpdatedEvent' => $baseDir . '/../lib/Events/SubscriptionUpdatedEvent.php',
273273
'OCA\\DAV\\Exception\\ServerMaintenanceMode' => $baseDir . '/../lib/Exception/ServerMaintenanceMode.php',
274274
'OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException' => $baseDir . '/../lib/Exception/UnsupportedLimitOnInitialSyncException.php',
275-
'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => $baseDir . '/../lib/Files/BrowserErrorPagePlugin.php',
275+
'OCA\\DAV\\Files\\ErrorPagePlugin' => $baseDir . '/../lib/Files/ErrorPagePlugin.php',
276276
'OCA\\DAV\\Files\\FileSearchBackend' => $baseDir . '/../lib/Files/FileSearchBackend.php',
277277
'OCA\\DAV\\Files\\FilesHome' => $baseDir . '/../lib/Files/FilesHome.php',
278278
'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
@@ -287,7 +287,7 @@ class ComposerStaticInitDAV
287287
'OCA\\DAV\\Events\\SubscriptionUpdatedEvent' => __DIR__ . '/..' . '/../lib/Events/SubscriptionUpdatedEvent.php',
288288
'OCA\\DAV\\Exception\\ServerMaintenanceMode' => __DIR__ . '/..' . '/../lib/Exception/ServerMaintenanceMode.php',
289289
'OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException' => __DIR__ . '/..' . '/../lib/Exception/UnsupportedLimitOnInitialSyncException.php',
290-
'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/BrowserErrorPagePlugin.php',
290+
'OCA\\DAV\\Files\\ErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/ErrorPagePlugin.php',
291291
'OCA\\DAV\\Files\\FileSearchBackend' => __DIR__ . '/..' . '/../lib/Files/FileSearchBackend.php',
292292
'OCA\\DAV\\Files\\FilesHome' => __DIR__ . '/..' . '/../lib/Files/FilesHome.php',
293293
'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
@@ -10,7 +10,7 @@
1010
use OCA\DAV\AppInfo\PluginManager;
1111
use OCA\DAV\CalDAV\DefaultCalendarValidator;
1212
use OCA\DAV\DAV\ViewOnlyPlugin;
13-
use OCA\DAV\Files\BrowserErrorPagePlugin;
13+
use OCA\DAV\Files\ErrorPagePlugin;
1414
use OCP\EventDispatcher\IEventDispatcher;
1515
use OCP\Files\Folder;
1616
use OCP\Files\IFilenameValidator;
@@ -98,9 +98,7 @@ public function createServer(string $baseUri,
9898
$server->addPlugin(new \OCA\DAV\Connector\Sabre\FakeLockerPlugin());
9999
}
100100

101-
if (BrowserErrorPagePlugin::isBrowserRequest($this->request)) {
102-
$server->addPlugin(new BrowserErrorPagePlugin());
103-
}
101+
$server->addPlugin(new ErrorPagePlugin($this->request, $this->config));
104102

105103
// wait with registering these until auth is handled and the filesystem is setup
106104
$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
@@ -7,17 +7,22 @@
77
*/
88
namespace OCA\DAV\Files;
99

10-
use OC\AppFramework\Http\Request;
1110
use OC_Template;
1211
use OCP\AppFramework\Http\ContentSecurityPolicy;
12+
use OCP\IConfig;
1313
use OCP\IRequest;
1414
use Sabre\DAV\Exception;
1515
use Sabre\DAV\Server;
1616
use Sabre\DAV\ServerPlugin;
1717

18-
class BrowserErrorPagePlugin extends ServerPlugin {
19-
/** @var Server */
20-
private $server;
18+
class ErrorPagePlugin extends ServerPlugin {
19+
private ?Server $server = null;
20+
21+
public function __construct(
22+
private IRequest $request,
23+
private IConfig $config,
24+
) {
25+
}
2126

2227
/**
2328
* This initializes the plugin.
@@ -26,35 +31,12 @@ class BrowserErrorPagePlugin extends ServerPlugin {
2631
* addPlugin is called.
2732
*
2833
* This method should set up the required event subscriptions.
29-
*
30-
* @param Server $server
31-
* @return void
3234
*/
33-
public function initialize(Server $server) {
35+
public function initialize(Server $server): void {
3436
$this->server = $server;
3537
$server->on('exception', [$this, 'logException'], 1000);
3638
}
3739

38-
/**
39-
* @param IRequest $request
40-
* @return bool
41-
*/
42-
public static function isBrowserRequest(IRequest $request) {
43-
if ($request->getMethod() !== 'GET') {
44-
return false;
45-
}
46-
return $request->isUserAgent([
47-
Request::USER_AGENT_IE,
48-
Request::USER_AGENT_MS_EDGE,
49-
Request::USER_AGENT_CHROME,
50-
Request::USER_AGENT_FIREFOX,
51-
Request::USER_AGENT_SAFARI,
52-
]);
53-
}
54-
55-
/**
56-
* @param \Throwable $ex
57-
*/
5840
public function logException(\Throwable $ex): void {
5941
if ($ex instanceof Exception) {
6042
$httpCode = $ex->getHTTPCode();
@@ -65,7 +47,7 @@ public function logException(\Throwable $ex): void {
6547
}
6648
$this->server->httpResponse->addHeaders($headers);
6749
$this->server->httpResponse->setStatus($httpCode);
68-
$body = $this->generateBody($httpCode);
50+
$body = $this->generateBody($ex, $httpCode);
6951
$this->server->httpResponse->setBody($body);
7052
$csp = new ContentSecurityPolicy();
7153
$this->server->httpResponse->addHeader('Content-Security-Policy', $csp->buildPolicy());
@@ -76,18 +58,32 @@ public function logException(\Throwable $ex): void {
7658
* @codeCoverageIgnore
7759
* @return bool|string
7860
*/
79-
public function generateBody(int $httpCode) {
80-
$request = \OC::$server->getRequest();
81-
82-
$templateName = 'exception';
83-
if ($httpCode === 403 || $httpCode === 404) {
84-
$templateName = (string)$httpCode;
61+
public function generateBody(\Throwable $ex, int $httpCode): mixed {
62+
if ($this->acceptHtml()) {
63+
$templateName = 'exception';
64+
$renderAs = 'guest';
65+
if ($httpCode === 403 || $httpCode === 404) {
66+
$templateName = (string)$httpCode;
67+
}
68+
} else {
69+
$templateName = 'xml_exception';
70+
$renderAs = null;
71+
$this->server->httpResponse->setHeader('Content-Type', 'application/xml; charset=utf-8');
8572
}
8673

87-
$content = new OC_Template('core', $templateName, 'guest');
74+
$debug = $this->config->getSystemValueBool('debug', false);
75+
76+
$content = new OC_Template('core', $templateName, $renderAs);
8877
$content->assign('title', $this->server->httpResponse->getStatusText());
89-
$content->assign('remoteAddr', $request->getRemoteAddress());
90-
$content->assign('requestID', $request->getId());
78+
$content->assign('remoteAddr', $this->request->getRemoteAddress());
79+
$content->assign('requestID', $this->request->getId());
80+
$content->assign('debugMode', $debug);
81+
$content->assign('errorClass', get_class($ex));
82+
$content->assign('errorMsg', $ex->getMessage());
83+
$content->assign('errorCode', $ex->getCode());
84+
$content->assign('file', $ex->getFile());
85+
$content->assign('line', $ex->getLine());
86+
$content->assign('exception', $ex);
9187
return $content->fetchPage();
9288
}
9389

@@ -98,4 +94,14 @@ public function sendResponse() {
9894
$this->server->sapi->sendResponse($this->server->httpResponse);
9995
exit();
10096
}
97+
98+
private function acceptHtml(): bool {
99+
foreach (explode(',', $this->request->getHeader('Accept')) as $part) {
100+
$subparts = explode(';', $part);
101+
if (str_ends_with($subparts[0], '/html')) {
102+
return true;
103+
}
104+
}
105+
return false;
106+
}
101107
}

‎apps/dav/lib/Server.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
use OCA\DAV\DAV\ViewOnlyPlugin;
4444
use OCA\DAV\Events\SabrePluginAddEvent;
4545
use OCA\DAV\Events\SabrePluginAuthInitEvent;
46-
use OCA\DAV\Files\BrowserErrorPagePlugin;
46+
use OCA\DAV\Files\ErrorPagePlugin;
4747
use OCA\DAV\Files\LazySearchBackend;
4848
use OCA\DAV\Profiler\ProfilerPlugin;
4949
use OCA\DAV\Provisioning\Apple\AppleProvisioningPlugin;
@@ -221,9 +221,7 @@ public function __construct(IRequest $request, string $baseUri) {
221221
$this->server->addPlugin(new FakeLockerPlugin());
222222
}
223223

224-
if (BrowserErrorPagePlugin::isBrowserRequest($request)) {
225-
$this->server->addPlugin(new BrowserErrorPagePlugin());
226-
}
224+
$this->server->addPlugin(new ErrorPagePlugin($this->request, \OC::$server->getConfig()));
227225

228226
$lazySearchBackend = new LazySearchBackend();
229227
$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
@@ -2701,7 +2701,7 @@
27012701
<callback>prepostcondition</callback>
27022702
<arg>
27032703
<name>error</name>
2704-
<value>{DAV:}valid-sync-token</value>
2704+
<value>{http://sabredav.org/ns}exception</value>
27052705
</arg>
27062706
<arg>
27072707
<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
@@ -7,20 +7,20 @@
77
*/
88
namespace OCA\DAV\Tests\unit\DAV;
99

10-
use OCA\DAV\Files\BrowserErrorPagePlugin;
10+
use OCA\DAV\Files\ErrorPagePlugin;
1111
use Sabre\DAV\Exception\NotFound;
1212
use Sabre\HTTP\Response;
1313

14-
class BrowserErrorPagePluginTest extends \Test\TestCase {
14+
class ErrorPagePluginTest extends \Test\TestCase {
1515

1616
/**
1717
* @dataProvider providesExceptions
1818
* @param $expectedCode
1919
* @param $exception
2020
*/
2121
public function test($expectedCode, $exception): void {
22-
/** @var BrowserErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */
23-
$plugin = $this->getMockBuilder(BrowserErrorPagePlugin::class)->setMethods(['sendResponse', 'generateBody'])->getMock();
22+
/** @var ErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */
23+
$plugin = $this->getMockBuilder(ErrorPagePlugin::class)->disableOriginalConstructor()->setMethods(['sendResponse', 'generateBody'])->getMock();
2424
$plugin->expects($this->once())->method('generateBody')->willReturn(':boom:');
2525
$plugin->expects($this->once())->method('sendResponse');
2626
/** @var \Sabre\DAV\Server | \PHPUnit\Framework\MockObject\MockObject $server */

‎build/integration/dav_features/caldav.feature‎

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

1111
Scenario: Accessing a not shared calendar of another user
1212
Given user "user0" exists
1313
Given "admin" creates a calendar named "MyCalendar"
1414
Given The CalDAV HTTP status code should be "201"
1515
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/"
1616
Then The CalDAV HTTP status code should be "404"
17-
And The exception is "Sabre\DAV\Exception\NotFound"
18-
And The error message is "Calendar with name 'MyCalendar' could not be found"
17+
And The exception is "Internal Server Error"
18+
And The error message is ""
1919

2020
Scenario: Accessing a not shared calendar of another user via the legacy endpoint
2121
Given user "user0" exists
2222
Given "admin" creates a calendar named "MyCalendar"
2323
Given The CalDAV HTTP status code should be "201"
2424
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/caldav/calendars/"
2525
Then The CalDAV HTTP status code should be "404"
26-
And The exception is "Sabre\DAV\Exception\NotFound"
27-
And The error message is "Calendar with name 'MyCalendar' could not be found"
26+
And The exception is "Internal Server Error"
27+
And The error message is ""
2828

2929
Scenario: Accessing a not existing calendar of another user
3030
Given user "user0" exists
3131
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/"
3232
Then The CalDAV HTTP status code should be "404"
33-
And The exception is "Sabre\DAV\Exception\NotFound"
34-
And The error message is "Node with name 'MyCalendar' could not be found"
33+
And The exception is "Internal Server Error"
34+
And The error message is ""
3535

3636
Scenario: Accessing a not existing calendar of another user via the legacy endpoint
3737
Given user "user0" exists
3838
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/caldav/calendars/"
3939
Then The CalDAV HTTP status code should be "404"
40-
And The exception is "Sabre\DAV\Exception\NotFound"
41-
And The error message is "Node with name 'MyCalendar' could not be found"
40+
And The exception is "Internal Server Error"
41+
And The error message is ""
4242

4343
Scenario: Accessing a not existing calendar of myself
4444
Given user "user0" exists
4545
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/"
4646
Then The CalDAV HTTP status code should be "404"
47-
And The exception is "Sabre\DAV\Exception\NotFound"
48-
And The error message is "Node with name 'MyCalendar' could not be found"
47+
And The exception is "Internal Server Error"
48+
And The error message is ""
4949

5050
Scenario: Creating a new calendar
5151
When "admin" creates a calendar named "MyCalendar"
@@ -66,17 +66,17 @@ Feature: caldav
6666
Given user "user0" exists
6767
When "user0" sends a create calendar request to "admin/MyCalendar2" on the endpoint "/remote.php/dav/calendars/"
6868
Then The CalDAV HTTP status code should be "404"
69-
And The exception is "Sabre\DAV\Exception\NotFound"
70-
And The error message is "Node with name 'admin' could not be found"
69+
And The exception is "Internal Server Error"
70+
And The error message is ""
7171

7272
Scenario: Create calendar request for existing calendar of another user
7373
Given user "user0" exists
7474
When "admin" creates a calendar named "MyCalendar2"
7575
Then The CalDAV HTTP status code should be "201"
7676
When "user0" sends a create calendar request to "admin/MyCalendar2" on the endpoint "/remote.php/dav/calendars/"
7777
Then The CalDAV HTTP status code should be "404"
78-
And The exception is "Sabre\DAV\Exception\NotFound"
79-
And The error message is "Node with name 'admin' could not be found"
78+
And The exception is "Internal Server Error"
79+
And The error message is ""
8080

8181
Scenario: Update a principal's schedule-default-calendar-URL
8282
Given user "user0" exists

‎build/integration/dav_features/carddav.feature‎

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

1010
Scenario: Accessing a not shared addressbook of another user
1111
Given user "user0" exists
1212
Given "admin" creates an addressbook named "MyAddressbook" with statuscode "201"
1313
When "user0" requests addressbook "admin/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/dav/addressbooks/users/"
14-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
15-
And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found"
14+
And The CardDAV exception is "Internal Server Error"
15+
And The CardDAV error message is ""
1616

1717
Scenario: Accessing a not existing addressbook of another user via legacy endpoint
1818
Given user "user0" exists
1919
When "admin" requests addressbook "user0/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/carddav/addressbooks/"
20-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
21-
And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found"
20+
And The CardDAV exception is "Internal Server Error"
21+
And The CardDAV error message is ""
2222

2323
Scenario: Accessing a not shared addressbook of another user via legacy endpoint
2424
Given user "user0" exists
2525
Given "admin" creates an addressbook named "MyAddressbook" with statuscode "201"
2626
When "user0" requests addressbook "admin/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/carddav/addressbooks/"
27-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
28-
And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found"
27+
And The CardDAV exception is "Internal Server Error"
28+
And The CardDAV error message is ""
2929

3030
Scenario: Accessing a not existing addressbook of myself
3131
Given user "user0" exists
3232
When "user0" requests addressbook "admin/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/dav/addressbooks/users/"
33-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
34-
And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found"
33+
And The CardDAV exception is "Internal Server Error"
34+
And The CardDAV error message is ""
3535

3636
Scenario: Creating a new addressbook
3737
When "admin" creates an addressbook named "MyAddressbook" with statuscode "201"
@@ -69,13 +69,13 @@ Feature: carddav
6969
Given user "user0" exists
7070
When "user0" sends a create addressbook request to "admin/MyAddressbook2" on the endpoint "/remote.php/dav/addressbooks/"
7171
Then The CardDAV HTTP status code should be "404"
72-
And The CardDAV exception is "Sabre\DAV\Exception\NotFound"
73-
And The CardDAV error message is "File not found: admin in 'addressbooks'"
72+
And The CardDAV exception is "Internal Server Error"
73+
And The CardDAV error message is ""
7474

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

0 commit comments

Comments
 (0)