Skip to content

Commit 1abe9de

Browse files
miaulalalamiaulalala
authored
Merge pull request #49004 from nextcloud/bug/48678/restore-dav-error-response
Bug/48678/restore dav error response
2 parents aa393aa + ca3733d commit 1abe9de

File tree

10 files changed

+148
-69
lines changed

10 files changed

+148
-69
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@
276276
'OCA\\DAV\\Events\\SubscriptionUpdatedEvent' => $baseDir . '/../lib/Events/SubscriptionUpdatedEvent.php',
277277
'OCA\\DAV\\Exception\\ServerMaintenanceMode' => $baseDir . '/../lib/Exception/ServerMaintenanceMode.php',
278278
'OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException' => $baseDir . '/../lib/Exception/UnsupportedLimitOnInitialSyncException.php',
279-
'OCA\\DAV\\Files\\ErrorPagePlugin' => $baseDir . '/../lib/Files/ErrorPagePlugin.php',
279+
'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => $baseDir . '/../lib/Files/BrowserErrorPagePlugin.php',
280280
'OCA\\DAV\\Files\\FileSearchBackend' => $baseDir . '/../lib/Files/FileSearchBackend.php',
281281
'OCA\\DAV\\Files\\FilesHome' => $baseDir . '/../lib/Files/FilesHome.php',
282282
'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
@@ -291,7 +291,7 @@ class ComposerStaticInitDAV
291291
'OCA\\DAV\\Events\\SubscriptionUpdatedEvent' => __DIR__ . '/..' . '/../lib/Events/SubscriptionUpdatedEvent.php',
292292
'OCA\\DAV\\Exception\\ServerMaintenanceMode' => __DIR__ . '/..' . '/../lib/Exception/ServerMaintenanceMode.php',
293293
'OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException' => __DIR__ . '/..' . '/../lib/Exception/UnsupportedLimitOnInitialSyncException.php',
294-
'OCA\\DAV\\Files\\ErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/ErrorPagePlugin.php',
294+
'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/BrowserErrorPagePlugin.php',
295295
'OCA\\DAV\\Files\\FileSearchBackend' => __DIR__ . '/..' . '/../lib/Files/FileSearchBackend.php',
296296
'OCA\\DAV\\Files\\FilesHome' => __DIR__ . '/..' . '/../lib/Files/FilesHome.php',
297297
'OCA\\DAV\\Files\\LazySearchBackend' => __DIR__ . '/..' . '/../lib/Files/LazySearchBackend.php',

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

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
*/
88
namespace OCA\DAV\Connector\Sabre;
99

10+
use Sabre\DAV\Exception;
11+
use Sabre\DAV\Version;
12+
1013
/**
1114
* Class \OCA\DAV\Connector\Sabre\Server
1215
*
@@ -26,9 +29,11 @@ public function __construct($treeOrNode = null) {
2629
$this->enablePropfindDepthInfinity = true;
2730
}
2831

29-
// Copied from 3rdparty/sabre/dav/lib/DAV/Server.php
30-
// Should be them exact same without the exception output.
31-
public function start(): void {
32+
/**
33+
*
34+
* @return void
35+
*/
36+
public function start() {
3237
try {
3338
// If nginx (pre-1.2) is used as a proxy server, and SabreDAV as an
3439
// origin, we must make sure we send back HTTP/1.0 if this was
@@ -42,10 +47,74 @@ public function start(): void {
4247
$this->httpRequest->setBaseUrl($this->getBaseUri());
4348
$this->invokeMethod($this->httpRequest, $this->httpResponse);
4449
} catch (\Throwable $e) {
50+
if ($e instanceof \TypeError) {
51+
/*
52+
* The TypeError includes the file path where the error occurred,
53+
* potentially revealing the installation directory.
54+
*
55+
* By re-throwing the exception, we ensure that the
56+
* default exception handler processes it.
57+
*/
58+
throw $e;
59+
}
60+
4561
try {
4662
$this->emit('exception', [$e]);
4763
} catch (\Exception $ignore) {
4864
}
65+
66+
$DOM = new \DOMDocument('1.0', 'utf-8');
67+
$DOM->formatOutput = true;
68+
69+
$error = $DOM->createElementNS('DAV:', 'd:error');
70+
$error->setAttribute('xmlns:s', self::NS_SABREDAV);
71+
$DOM->appendChild($error);
72+
73+
$h = function ($v) {
74+
return htmlspecialchars((string)$v, ENT_NOQUOTES, 'UTF-8');
75+
};
76+
77+
if (self::$exposeVersion) {
78+
$error->appendChild($DOM->createElement('s:sabredav-version', $h(Version::VERSION)));
79+
}
80+
81+
$error->appendChild($DOM->createElement('s:exception', $h(get_class($e))));
82+
$error->appendChild($DOM->createElement('s:message', $h($e->getMessage())));
83+
if ($this->debugExceptions) {
84+
$error->appendChild($DOM->createElement('s:file', $h($e->getFile())));
85+
$error->appendChild($DOM->createElement('s:line', $h($e->getLine())));
86+
$error->appendChild($DOM->createElement('s:code', $h($e->getCode())));
87+
$error->appendChild($DOM->createElement('s:stacktrace', $h($e->getTraceAsString())));
88+
}
89+
90+
if ($this->debugExceptions) {
91+
$previous = $e;
92+
while ($previous = $previous->getPrevious()) {
93+
$xPrevious = $DOM->createElement('s:previous-exception');
94+
$xPrevious->appendChild($DOM->createElement('s:exception', $h(get_class($previous))));
95+
$xPrevious->appendChild($DOM->createElement('s:message', $h($previous->getMessage())));
96+
$xPrevious->appendChild($DOM->createElement('s:file', $h($previous->getFile())));
97+
$xPrevious->appendChild($DOM->createElement('s:line', $h($previous->getLine())));
98+
$xPrevious->appendChild($DOM->createElement('s:code', $h($previous->getCode())));
99+
$xPrevious->appendChild($DOM->createElement('s:stacktrace', $h($previous->getTraceAsString())));
100+
$error->appendChild($xPrevious);
101+
}
102+
}
103+
104+
if ($e instanceof Exception) {
105+
$httpCode = $e->getHTTPCode();
106+
$e->serialize($this, $error);
107+
$headers = $e->getHTTPHeaders($this);
108+
} else {
109+
$httpCode = 500;
110+
$headers = [];
111+
}
112+
$headers['Content-Type'] = 'application/xml; charset=utf-8';
113+
114+
$this->httpResponse->setStatus($httpCode);
115+
$this->httpResponse->setHeaders($headers);
116+
$this->httpResponse->setBody($DOM->saveXML());
117+
$this->sapi->sendResponse($this->httpResponse);
49118
}
50119
}
51120
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
use OCA\DAV\CalDAV\DefaultCalendarValidator;
1313
use OCA\DAV\DAV\CustomPropertiesBackend;
1414
use OCA\DAV\DAV\ViewOnlyPlugin;
15-
use OCA\DAV\Files\ErrorPagePlugin;
15+
use OCA\DAV\Files\BrowserErrorPagePlugin;
1616
use OCA\Theming\ThemingDefaults;
1717
use OCP\EventDispatcher\IEventDispatcher;
1818
use OCP\Files\Folder;
@@ -90,7 +90,9 @@ public function createServer(string $baseUri,
9090
$server->addPlugin(new FakeLockerPlugin());
9191
}
9292

93-
$server->addPlugin(new ErrorPagePlugin($this->request, $this->config));
93+
if (BrowserErrorPagePlugin::isBrowserRequest($this->request)) {
94+
$server->addPlugin(new BrowserErrorPagePlugin());
95+
}
9496

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

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

Lines changed: 39 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,17 @@
77
*/
88
namespace OCA\DAV\Files;
99

10+
use OC\AppFramework\Http\Request;
1011
use OC_Template;
1112
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 ErrorPagePlugin extends ServerPlugin {
19-
private ?Server $server = null;
20-
21-
public function __construct(
22-
private IRequest $request,
23-
private IConfig $config,
24-
) {
25-
}
18+
class BrowserErrorPagePlugin extends ServerPlugin {
19+
/** @var Server */
20+
private $server;
2621

2722
/**
2823
* This initializes the plugin.
@@ -31,12 +26,35 @@ public function __construct(
3126
* addPlugin is called.
3227
*
3328
* This method should set up the required event subscriptions.
29+
*
30+
* @param Server $server
31+
* @return void
3432
*/
35-
public function initialize(Server $server): void {
33+
public function initialize(Server $server) {
3634
$this->server = $server;
3735
$server->on('exception', [$this, 'logException'], 1000);
3836
}
3937

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+
*/
4058
public function logException(\Throwable $ex): void {
4159
if ($ex instanceof Exception) {
4260
$httpCode = $ex->getHTTPCode();
@@ -47,7 +65,7 @@ public function logException(\Throwable $ex): void {
4765
}
4866
$this->server->httpResponse->addHeaders($headers);
4967
$this->server->httpResponse->setStatus($httpCode);
50-
$body = $this->generateBody($ex, $httpCode);
68+
$body = $this->generateBody($httpCode);
5169
$this->server->httpResponse->setBody($body);
5270
$csp = new ContentSecurityPolicy();
5371
$this->server->httpResponse->addHeader('Content-Security-Policy', $csp->buildPolicy());
@@ -58,32 +76,18 @@ public function logException(\Throwable $ex): void {
5876
* @codeCoverageIgnore
5977
* @return bool|string
6078
*/
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');
72-
}
79+
public function generateBody(int $httpCode) {
80+
$request = \OC::$server->getRequest();
7381

74-
$debug = $this->config->getSystemValueBool('debug', false);
82+
$templateName = 'exception';
83+
if ($httpCode === 403 || $httpCode === 404) {
84+
$templateName = (string)$httpCode;
85+
}
7586

76-
$content = new OC_Template('core', $templateName, $renderAs);
87+
$content = new OC_Template('core', $templateName, 'guest');
7788
$content->assign('title', $this->server->httpResponse->getStatusText());
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);
89+
$content->assign('remoteAddr', $request->getRemoteAddress());
90+
$content->assign('requestID', $request->getId());
8791
return $content->fetchPage();
8892
}
8993

@@ -92,15 +96,6 @@ public function generateBody(\Throwable $ex, int $httpCode): mixed {
9296
*/
9397
public function sendResponse() {
9498
$this->server->sapi->sendResponse($this->server->httpResponse);
95-
}
96-
97-
private function acceptHtml(): bool {
98-
foreach (explode(',', $this->request->getHeader('Accept')) as $part) {
99-
$subparts = explode(';', $part);
100-
if (str_ends_with($subparts[0], '/html')) {
101-
return true;
102-
}
103-
}
104-
return false;
99+
exit();
105100
}
106101
}

‎apps/dav/lib/Server.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
use OCA\DAV\DAV\ViewOnlyPlugin;
5555
use OCA\DAV\Events\SabrePluginAddEvent;
5656
use OCA\DAV\Events\SabrePluginAuthInitEvent;
57-
use OCA\DAV\Files\ErrorPagePlugin;
57+
use OCA\DAV\Files\BrowserErrorPagePlugin;
5858
use OCA\DAV\Files\FileSearchBackend;
5959
use OCA\DAV\Files\LazySearchBackend;
6060
use OCA\DAV\Profiler\ProfilerPlugin;
@@ -244,7 +244,9 @@ public function __construct(
244244
$this->server->addPlugin(new FakeLockerPlugin());
245245
}
246246

247-
$this->server->addPlugin(new ErrorPagePlugin($this->request, \OC::$server->getConfig()));
247+
if (BrowserErrorPagePlugin::isBrowserRequest($request)) {
248+
$this->server->addPlugin(new BrowserErrorPagePlugin());
249+
}
248250

249251
$lazySearchBackend = new LazySearchBackend();
250252
$this->server->addPlugin(new SearchPlugin($lazySearchBackend));

‎apps/dav/tests/testsuits/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>{http://sabredav.org/ns}exception</value>
2704+
<value>{DAV:}valid-sync-token</value>
27052705
</arg>
27062706
<arg>
27072707
<name>ignoreextras</name>

apps/dav/tests/unit/DAV/ErrorPagePluginTest.php renamed to apps/dav/tests/unit/DAV/BrowserErrorPagePluginTest.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\ErrorPagePlugin;
10+
use OCA\DAV\Files\BrowserErrorPagePlugin;
1111
use Sabre\DAV\Exception\NotFound;
1212
use Sabre\HTTP\Response;
1313

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

1616
/**
1717
* @dataProvider providesExceptions
1818
* @param $expectedCode
1919
* @param $exception
2020
*/
2121
public function test($expectedCode, $exception): void {
22-
/** @var ErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */
23-
$plugin = $this->getMockBuilder(ErrorPagePlugin::class)->disableOriginalConstructor()->setMethods(['sendResponse', 'generateBody'])->getMock();
22+
/** @var BrowserErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */
23+
$plugin = $this->getMockBuilder(BrowserErrorPagePlugin::class)->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: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,17 @@ 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 "Internal Server Error"
8+
And The exception is "Sabre\DAV\Exception\NotFound"
9+
And The error message is "Node with name 'MyCalendar' could not be found"
910

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

1820
Scenario: Accessing a not shared calendar of another user via the legacy endpoint
1921
Given user "user0" exists
@@ -28,7 +30,8 @@ Feature: caldav
2830
Given user "user0" exists
2931
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/"
3032
Then The CalDAV HTTP status code should be "404"
31-
And The exception is "Internal Server Error"
33+
And The exception is "Sabre\DAV\Exception\NotFound"
34+
And The error message is "Node with name 'MyCalendar' could not be found"
3235

3336
Scenario: Accessing a not existing calendar of another user via the legacy endpoint
3437
Given user "user0" exists
@@ -41,7 +44,8 @@ Feature: caldav
4144
Given user "user0" exists
4245
When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/"
4346
Then The CalDAV HTTP status code should be "404"
44-
And The exception is "Internal Server Error"
47+
And The exception is "Sabre\DAV\Exception\NotFound"
48+
And The error message is "Node with name 'MyCalendar' could not be found"
4549

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

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

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

0 commit comments

Comments
 (0)