diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 4a14709454bf5..78c4ef645bfe6 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -247,7 +247,7 @@ 'OCA\\DAV\\Events\\SubscriptionUpdatedEvent' => $baseDir . '/../lib/Events/SubscriptionUpdatedEvent.php', 'OCA\\DAV\\Exception\\ServerMaintenanceMode' => $baseDir . '/../lib/Exception/ServerMaintenanceMode.php', 'OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException' => $baseDir . '/../lib/Exception/UnsupportedLimitOnInitialSyncException.php', - 'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => $baseDir . '/../lib/Files/BrowserErrorPagePlugin.php', + 'OCA\\DAV\\Files\\ErrorPagePlugin' => $baseDir . '/../lib/Files/ErrorPagePlugin.php', 'OCA\\DAV\\Files\\FileSearchBackend' => $baseDir . '/../lib/Files/FileSearchBackend.php', 'OCA\\DAV\\Files\\FilesHome' => $baseDir . '/../lib/Files/FilesHome.php', 'OCA\\DAV\\Files\\LazySearchBackend' => $baseDir . '/../lib/Files/LazySearchBackend.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index c1bfc65179f4c..31206caa903c0 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -262,7 +262,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Events\\SubscriptionUpdatedEvent' => __DIR__ . '/..' . '/../lib/Events/SubscriptionUpdatedEvent.php', 'OCA\\DAV\\Exception\\ServerMaintenanceMode' => __DIR__ . '/..' . '/../lib/Exception/ServerMaintenanceMode.php', 'OCA\\DAV\\Exception\\UnsupportedLimitOnInitialSyncException' => __DIR__ . '/..' . '/../lib/Exception/UnsupportedLimitOnInitialSyncException.php', - 'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/BrowserErrorPagePlugin.php', + 'OCA\\DAV\\Files\\ErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/ErrorPagePlugin.php', 'OCA\\DAV\\Files\\FileSearchBackend' => __DIR__ . '/..' . '/../lib/Files/FileSearchBackend.php', 'OCA\\DAV\\Files\\FilesHome' => __DIR__ . '/..' . '/../lib/Files/FilesHome.php', 'OCA\\DAV\\Files\\LazySearchBackend' => __DIR__ . '/..' . '/../lib/Files/LazySearchBackend.php', diff --git a/apps/dav/lib/Connector/Sabre/Server.php b/apps/dav/lib/Connector/Sabre/Server.php index 6cf6fa954c8db..0831a47b49198 100644 --- a/apps/dav/lib/Connector/Sabre/Server.php +++ b/apps/dav/lib/Connector/Sabre/Server.php @@ -43,4 +43,27 @@ public function __construct($treeOrNode = null) { self::$exposeVersion = false; $this->enablePropfindDepthInfinity = true; } + + // Copied from 3rdparty/sabre/dav/lib/DAV/Server.php + // Should be them exact same without the exception output. + public function start(): void { + try { + // If nginx (pre-1.2) is used as a proxy server, and SabreDAV as an + // origin, we must make sure we send back HTTP/1.0 if this was + // requested. + // This is mainly because nginx doesn't support Chunked Transfer + // Encoding, and this forces the webserver SabreDAV is running on, + // to buffer entire responses to calculate Content-Length. + $this->httpResponse->setHTTPVersion($this->httpRequest->getHTTPVersion()); + + // Setting the base url + $this->httpRequest->setBaseUrl($this->getBaseUri()); + $this->invokeMethod($this->httpRequest, $this->httpResponse); + } catch (\Throwable $e) { + try { + $this->emit('exception', [$e]); + } catch (\Exception $ignore) { + } + } + } } diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 755d13f837184..5725e42b04f84 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -34,7 +34,7 @@ use OCP\Files\Folder; use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\DAV\ViewOnlyPlugin; -use OCA\DAV\Files\BrowserErrorPagePlugin; +use OCA\DAV\Files\ErrorPagePlugin; use OCP\Files\Mount\IMountManager; use OCP\IConfig; use OCP\IDBConnection; @@ -120,9 +120,7 @@ public function createServer(string $baseUri, $server->addPlugin(new \OCA\DAV\Connector\Sabre\FakeLockerPlugin()); } - if (BrowserErrorPagePlugin::isBrowserRequest($this->request)) { - $server->addPlugin(new BrowserErrorPagePlugin()); - } + $server->addPlugin(new ErrorPagePlugin($this->request, $this->config)); // wait with registering these until auth is handled and the filesystem is setup $server->on('beforeMethod:*', function () use ($server, $objectTree, $viewCallBack) { diff --git a/apps/dav/lib/Files/BrowserErrorPagePlugin.php b/apps/dav/lib/Files/ErrorPagePlugin.php similarity index 58% rename from apps/dav/lib/Files/BrowserErrorPagePlugin.php rename to apps/dav/lib/Files/ErrorPagePlugin.php index b3ce591bd4a8e..9f834cfbcb542 100644 --- a/apps/dav/lib/Files/BrowserErrorPagePlugin.php +++ b/apps/dav/lib/Files/ErrorPagePlugin.php @@ -24,17 +24,22 @@ */ namespace OCA\DAV\Files; -use OC\AppFramework\Http\Request; use OC_Template; use OCP\AppFramework\Http\ContentSecurityPolicy; +use OCP\IConfig; use OCP\IRequest; use Sabre\DAV\Exception; use Sabre\DAV\Server; use Sabre\DAV\ServerPlugin; -class BrowserErrorPagePlugin extends ServerPlugin { - /** @var Server */ - private $server; +class ErrorPagePlugin extends ServerPlugin { + private ?Server $server = null; + + public function __construct( + private IRequest $request, + private IConfig $config, + ) { + } /** * This initializes the plugin. @@ -43,36 +48,13 @@ class BrowserErrorPagePlugin extends ServerPlugin { * addPlugin is called. * * This method should set up the required event subscriptions. - * - * @param Server $server - * @return void */ - public function initialize(Server $server) { + public function initialize(Server $server): void { $this->server = $server; $server->on('exception', [$this, 'logException'], 1000); } - /** - * @param IRequest $request - * @return bool - */ - public static function isBrowserRequest(IRequest $request) { - if ($request->getMethod() !== 'GET') { - return false; - } - return $request->isUserAgent([ - Request::USER_AGENT_IE, - Request::USER_AGENT_MS_EDGE, - Request::USER_AGENT_CHROME, - Request::USER_AGENT_FIREFOX, - Request::USER_AGENT_SAFARI, - ]); - } - - /** - * @param \Exception $ex - */ - public function logException(\Exception $ex) { + public function logException(\Throwable $ex): void { if ($ex instanceof Exception) { $httpCode = $ex->getHTTPCode(); $headers = $ex->getHTTPHeaders($this->server); @@ -82,7 +64,7 @@ public function logException(\Exception $ex) { } $this->server->httpResponse->addHeaders($headers); $this->server->httpResponse->setStatus($httpCode); - $body = $this->generateBody($httpCode); + $body = $this->generateBody($ex, $httpCode); $this->server->httpResponse->setBody($body); $csp = new ContentSecurityPolicy(); $this->server->httpResponse->addHeader('Content-Security-Policy', $csp->buildPolicy()); @@ -93,18 +75,32 @@ public function logException(\Exception $ex) { * @codeCoverageIgnore * @return bool|string */ - public function generateBody(int $httpCode) { - $request = \OC::$server->getRequest(); - - $templateName = 'exception'; - if ($httpCode === 403 || $httpCode === 404) { - $templateName = (string)$httpCode; + public function generateBody(\Throwable $ex, int $httpCode): mixed { + if ($this->acceptHtml()) { + $templateName = 'exception'; + $renderAs = 'guest'; + if ($httpCode === 403 || $httpCode === 404) { + $templateName = (string)$httpCode; + } + } else { + $templateName = 'xml_exception'; + $renderAs = null; + $this->server->httpResponse->setHeader('Content-Type', 'application/xml; charset=utf-8'); } - $content = new OC_Template('core', $templateName, 'guest'); + $debug = $this->config->getSystemValueBool('debug', false); + + $content = new OC_Template('core', $templateName, $renderAs); $content->assign('title', $this->server->httpResponse->getStatusText()); - $content->assign('remoteAddr', $request->getRemoteAddress()); - $content->assign('requestID', $request->getId()); + $content->assign('remoteAddr', $this->request->getRemoteAddress()); + $content->assign('requestID', $this->request->getId()); + $content->assign('debugMode', $debug); + $content->assign('errorClass', get_class($ex)); + $content->assign('errorMsg', $ex->getMessage()); + $content->assign('errorCode', $ex->getCode()); + $content->assign('file', $ex->getFile()); + $content->assign('line', $ex->getLine()); + $content->assign('exception', $ex); return $content->fetchPage(); } @@ -113,6 +109,15 @@ public function generateBody(int $httpCode) { */ public function sendResponse() { $this->server->sapi->sendResponse($this->server->httpResponse); - exit(); + } + + private function acceptHtml(): bool { + foreach (explode(',', $this->request->getHeader('Accept')) as $part) { + $subparts = explode(';', $part); + if (str_ends_with($subparts[0], '/html')) { + return true; + } + } + return false; } } diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index f599865f1f562..12f95ead2a554 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -69,7 +69,7 @@ use OCA\DAV\DAV\PublicAuth; use OCA\DAV\DAV\ViewOnlyPlugin; use OCA\DAV\Events\SabrePluginAuthInitEvent; -use OCA\DAV\Files\BrowserErrorPagePlugin; +use OCA\DAV\Files\ErrorPagePlugin; use OCA\DAV\Files\LazySearchBackend; use OCA\DAV\Profiler\ProfilerPlugin; use OCA\DAV\Provisioning\Apple\AppleProvisioningPlugin; @@ -244,9 +244,7 @@ public function __construct(IRequest $request, string $baseUri) { $this->server->addPlugin(new FakeLockerPlugin()); } - if (BrowserErrorPagePlugin::isBrowserRequest($request)) { - $this->server->addPlugin(new BrowserErrorPagePlugin()); - } + $this->server->addPlugin(new ErrorPagePlugin($this->request, \OC::$server->getConfig())); $lazySearchBackend = new LazySearchBackend(); $this->server->addPlugin(new SearchPlugin($lazySearchBackend)); diff --git a/apps/dav/tests/travis/caldavtest/tests/CalDAV/sync-report.xml b/apps/dav/tests/travis/caldavtest/tests/CalDAV/sync-report.xml index cf4fcde251f4d..388d9df841383 100644 --- a/apps/dav/tests/travis/caldavtest/tests/CalDAV/sync-report.xml +++ b/apps/dav/tests/travis/caldavtest/tests/CalDAV/sync-report.xml @@ -2712,7 +2712,7 @@ prepostcondition error - {DAV:}valid-sync-token + {http://sabredav.org/ns}exception ignoreextras diff --git a/apps/dav/tests/unit/DAV/BrowserErrorPagePluginTest.php b/apps/dav/tests/unit/DAV/ErrorPagePluginTest.php similarity index 86% rename from apps/dav/tests/unit/DAV/BrowserErrorPagePluginTest.php rename to apps/dav/tests/unit/DAV/ErrorPagePluginTest.php index b6ec05afd7875..3c87574e8d28f 100644 --- a/apps/dav/tests/unit/DAV/BrowserErrorPagePluginTest.php +++ b/apps/dav/tests/unit/DAV/ErrorPagePluginTest.php @@ -23,11 +23,11 @@ */ namespace OCA\DAV\Tests\unit\DAV; -use OCA\DAV\Files\BrowserErrorPagePlugin; +use OCA\DAV\Files\ErrorPagePlugin; use Sabre\DAV\Exception\NotFound; use Sabre\HTTP\Response; -class BrowserErrorPagePluginTest extends \Test\TestCase { +class ErrorPagePluginTest extends \Test\TestCase { /** * @dataProvider providesExceptions @@ -35,8 +35,8 @@ class BrowserErrorPagePluginTest extends \Test\TestCase { * @param $exception */ public function test($expectedCode, $exception): void { - /** @var BrowserErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */ - $plugin = $this->getMockBuilder(BrowserErrorPagePlugin::class)->setMethods(['sendResponse', 'generateBody'])->getMock(); + /** @var ErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */ + $plugin = $this->getMockBuilder(ErrorPagePlugin::class)->disableOriginalConstructor()->setMethods(['sendResponse', 'generateBody'])->getMock(); $plugin->expects($this->once())->method('generateBody')->willReturn(':boom:'); $plugin->expects($this->once())->method('sendResponse'); /** @var \Sabre\DAV\Server | \PHPUnit\Framework\MockObject\MockObject $server */ diff --git a/build/integration/features/caldav.feature b/build/integration/features/caldav.feature index fffdd89d36748..3e81a37cf671f 100644 --- a/build/integration/features/caldav.feature +++ b/build/integration/features/caldav.feature @@ -3,8 +3,7 @@ Feature: caldav Given user "user0" exists When "admin" requests calendar "user0/MyCalendar" on the endpoint "/remote.php/dav/calendars/" Then The CalDAV HTTP status code should be "404" - And The exception is "Sabre\DAV\Exception\NotFound" - And The error message is "Node with name 'MyCalendar' could not be found" + And The exception is "Internal Server Error" Scenario: Accessing a not shared calendar of another user Given user "user0" exists @@ -12,8 +11,7 @@ Feature: caldav Given The CalDAV HTTP status code should be "201" When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/" Then The CalDAV HTTP status code should be "404" - And The exception is "Sabre\DAV\Exception\NotFound" - And The error message is "Calendar with name 'MyCalendar' could not be found" + And The exception is "Internal Server Error" Scenario: Accessing a not shared calendar of another user via the legacy endpoint Given user "user0" exists @@ -28,8 +26,7 @@ Feature: caldav Given user "user0" exists When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/" Then The CalDAV HTTP status code should be "404" - And The exception is "Sabre\DAV\Exception\NotFound" - And The error message is "Node with name 'MyCalendar' could not be found" + And The exception is "Internal Server Error" Scenario: Accessing a not existing calendar of another user via the legacy endpoint Given user "user0" exists @@ -42,8 +39,7 @@ Feature: caldav Given user "user0" exists When "user0" requests calendar "admin/MyCalendar" on the endpoint "/remote.php/dav/calendars/" Then The CalDAV HTTP status code should be "404" - And The exception is "Sabre\DAV\Exception\NotFound" - And The error message is "Node with name 'MyCalendar' could not be found" + And The exception is "Internal Server Error" Scenario: Creating a new calendar When "admin" creates a calendar named "MyCalendar" @@ -64,8 +60,7 @@ Feature: caldav Given user "user0" exists When "user0" sends a create calendar request to "admin/MyCalendar2" on the endpoint "/remote.php/dav/calendars/" Then The CalDAV HTTP status code should be "404" - And The exception is "Sabre\DAV\Exception\NotFound" - And The error message is "Node with name 'admin' could not be found" + And The exception is "Internal Server Error" Scenario: Create calendar request for existing calendar of another user Given user "user0" exists @@ -73,5 +68,4 @@ Feature: caldav Then The CalDAV HTTP status code should be "201" When "user0" sends a create calendar request to "admin/MyCalendar2" on the endpoint "/remote.php/dav/calendars/" Then The CalDAV HTTP status code should be "404" - And The exception is "Sabre\DAV\Exception\NotFound" - And The error message is "Node with name 'admin' could not be found" + And The exception is "Internal Server Error" diff --git a/build/integration/features/carddav.feature b/build/integration/features/carddav.feature index 9c9df6ddd94be..15f1e95e73770 100644 --- a/build/integration/features/carddav.feature +++ b/build/integration/features/carddav.feature @@ -2,15 +2,13 @@ Feature: carddav Scenario: Accessing a not existing addressbook of another user Given user "user0" exists When "admin" requests addressbook "user0/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/dav/addressbooks/users/" - And The CardDAV exception is "Sabre\DAV\Exception\NotFound" - And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found" + And The CardDAV exception is "Internal Server Error" Scenario: Accessing a not shared addressbook of another user Given user "user0" exists Given "admin" creates an addressbook named "MyAddressbook" with statuscode "201" When "user0" requests addressbook "admin/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/dav/addressbooks/users/" - And The CardDAV exception is "Sabre\DAV\Exception\NotFound" - And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found" + And The CardDAV exception is "Internal Server Error" Scenario: Accessing a not existing addressbook of another user via legacy endpoint Given user "user0" exists @@ -28,8 +26,7 @@ Feature: carddav Scenario: Accessing a not existing addressbook of myself Given user "user0" exists When "user0" requests addressbook "admin/MyAddressbook" with statuscode "404" on the endpoint "/remote.php/dav/addressbooks/users/" - And The CardDAV exception is "Sabre\DAV\Exception\NotFound" - And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found" + And The CardDAV exception is "Internal Server Error" Scenario: Creating a new addressbook When "admin" creates an addressbook named "MyAddressbook" with statuscode "201" @@ -67,13 +64,11 @@ Feature: carddav Given user "user0" exists When "user0" sends a create addressbook request to "admin/MyAddressbook2" on the endpoint "/remote.php/dav/addressbooks/" Then The CardDAV HTTP status code should be "404" - And The CardDAV exception is "Sabre\DAV\Exception\NotFound" - And The CardDAV error message is "File not found: admin in 'addressbooks'" + And The CardDAV exception is "Internal Server Error" Scenario: Create addressbook request for existing addressbook of another user Given user "user0" exists When "admin" creates an addressbook named "MyAddressbook2" with statuscode "201" When "user0" sends a create addressbook request to "admin/MyAddressbook2" on the endpoint "/remote.php/dav/addressbooks/" Then The CardDAV HTTP status code should be "404" - And The CardDAV exception is "Sabre\DAV\Exception\NotFound" - And The CardDAV error message is "File not found: admin in 'addressbooks'" + And The CardDAV exception is "Internal Server Error" diff --git a/core/templates/exception.php b/core/templates/exception.php index d26e9ff4f94ed..324724e67ace2 100644 --- a/core/templates/exception.php +++ b/core/templates/exception.php @@ -4,20 +4,7 @@ style('core', ['styles', 'header']); -function print_exception(Throwable $e, \OCP\IL10N $l): void { - print_unescaped('
');
-	p($e->getTraceAsString());
-	print_unescaped('
'); - - if ($e->getPrevious() !== null) { - print_unescaped('
'); - print_unescaped('

'); - p($l->t('Previous')); - print_unescaped('

'); - - print_exception($e->getPrevious(), $l); - } -} +require_once __DIR__ . '/print_exception.php'; ?>
diff --git a/core/templates/print_exception.php b/core/templates/print_exception.php new file mode 100644 index 0000000000000..2def6d4e9d904 --- /dev/null +++ b/core/templates/print_exception.php @@ -0,0 +1,21 @@ +'); + p($e->getTraceAsString()); + print_unescaped(''); + + if ($e->getPrevious() !== null) { + print_unescaped('
'); + print_unescaped('

'); + p($l->t('Previous')); + print_unescaped('

'); + + print_exception($e->getPrevious(), $l); + } +} diff --git a/core/templates/print_xml_exception.php b/core/templates/print_xml_exception.php new file mode 100644 index 0000000000000..94452d8ae9d3f --- /dev/null +++ b/core/templates/print_xml_exception.php @@ -0,0 +1,16 @@ +getTraceAsString()); + + if ($e->getPrevious() !== null) { + print_unescaped(''); + print_exception($e->getPrevious(), $l); + print_unescaped(''); + } +} diff --git a/core/templates/xml_exception.php b/core/templates/xml_exception.php new file mode 100644 index 0000000000000..ba808c88595c2 --- /dev/null +++ b/core/templates/xml_exception.php @@ -0,0 +1,39 @@ +' . "\n"); +?> + + t('Internal Server Error')) ?> + + t('The server was unable to complete your request.')) ?> + t('If this happens again, please send the technical details below to the server administrator.')) ?> + t('More details can be found in the server log.')) ?> + + t('For more details see the documentation ↗.'))?>: + + + + + + + + + + + + + + + + + + + +