diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index bb4e6daf5183b..845f7cdc0d827 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -258,7 +258,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\\ErrorPagePlugin' => $baseDir . '/../lib/Files/ErrorPagePlugin.php', + 'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => $baseDir . '/../lib/Files/BrowserErrorPagePlugin.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 af38a1bc08073..723dd3692c32a 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -273,7 +273,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\\ErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/ErrorPagePlugin.php', + 'OCA\\DAV\\Files\\BrowserErrorPagePlugin' => __DIR__ . '/..' . '/../lib/Files/BrowserErrorPagePlugin.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 0831a47b49198..4930ce29cfdf7 100644 --- a/apps/dav/lib/Connector/Sabre/Server.php +++ b/apps/dav/lib/Connector/Sabre/Server.php @@ -25,6 +25,9 @@ */ namespace OCA\DAV\Connector\Sabre; +use Sabre\DAV\Exception; +use Sabre\DAV\Version; + /** * Class \OCA\DAV\Connector\Sabre\Server * @@ -44,9 +47,11 @@ public function __construct($treeOrNode = null) { $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 { + /** + * + * @return void + */ + public function start() { 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 @@ -59,11 +64,73 @@ public function start(): void { // Setting the base url $this->httpRequest->setBaseUrl($this->getBaseUri()); $this->invokeMethod($this->httpRequest, $this->httpResponse); + } catch (\Error $e) { + /* + * The TypeError includes the file path where the error occurred, + * potentially revealing the installation directory. + * + * By re-throwing the exception, we ensure that the + * default exception handler processes it. + */ + throw $e; } catch (\Throwable $e) { try { $this->emit('exception', [$e]); } catch (\Exception $ignore) { } + + $DOM = new \DOMDocument('1.0', 'utf-8'); + $DOM->formatOutput = true; + + $error = $DOM->createElementNS('DAV:', 'd:error'); + $error->setAttribute('xmlns:s', self::NS_SABREDAV); + $DOM->appendChild($error); + + $h = function ($v) { + return htmlspecialchars((string)$v, ENT_NOQUOTES, 'UTF-8'); + }; + + if (self::$exposeVersion) { + $error->appendChild($DOM->createElement('s:sabredav-version', $h(Version::VERSION))); + } + + $error->appendChild($DOM->createElement('s:exception', $h(get_class($e)))); + $error->appendChild($DOM->createElement('s:message', $h($e->getMessage()))); + if ($this->debugExceptions) { + $error->appendChild($DOM->createElement('s:file', $h($e->getFile()))); + $error->appendChild($DOM->createElement('s:line', $h($e->getLine()))); + $error->appendChild($DOM->createElement('s:code', $h($e->getCode()))); + $error->appendChild($DOM->createElement('s:stacktrace', $h($e->getTraceAsString()))); + } + + if ($this->debugExceptions) { + $previous = $e; + while ($previous = $previous->getPrevious()) { + $xPrevious = $DOM->createElement('s:previous-exception'); + $xPrevious->appendChild($DOM->createElement('s:exception', $h(get_class($previous)))); + $xPrevious->appendChild($DOM->createElement('s:message', $h($previous->getMessage()))); + $xPrevious->appendChild($DOM->createElement('s:file', $h($previous->getFile()))); + $xPrevious->appendChild($DOM->createElement('s:line', $h($previous->getLine()))); + $xPrevious->appendChild($DOM->createElement('s:code', $h($previous->getCode()))); + $xPrevious->appendChild($DOM->createElement('s:stacktrace', $h($previous->getTraceAsString()))); + $error->appendChild($xPrevious); + } + } + + if ($e instanceof Exception) { + $httpCode = $e->getHTTPCode(); + $e->serialize($this, $error); + $headers = $e->getHTTPHeaders($this); + } else { + $httpCode = 500; + $headers = []; + } + $headers['Content-Type'] = 'application/xml; charset=utf-8'; + + $this->httpResponse->setStatus($httpCode); + $this->httpResponse->setHeaders($headers); + $this->httpResponse->setBody($DOM->saveXML()); + $this->sapi->sendResponse($this->httpResponse); } } } diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 1af6f5c79f8e8..b67264fcb1431 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -31,9 +31,10 @@ */ namespace OCA\DAV\Connector\Sabre; +use OC\Files\View; use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\DAV\ViewOnlyPlugin; -use OCA\DAV\Files\ErrorPagePlugin; +use OCA\DAV\Files\BrowserErrorPagePlugin; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Folder; use OCP\Files\Mount\IMountManager; @@ -120,7 +121,9 @@ public function createServer(string $baseUri, $server->addPlugin(new \OCA\DAV\Connector\Sabre\FakeLockerPlugin()); } - $server->addPlugin(new ErrorPagePlugin($this->request, $this->config)); + if (BrowserErrorPagePlugin::isBrowserRequest($this->request)) { + $server->addPlugin(new BrowserErrorPagePlugin()); + } // 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/ErrorPagePlugin.php b/apps/dav/lib/Files/BrowserErrorPagePlugin.php similarity index 59% rename from apps/dav/lib/Files/ErrorPagePlugin.php rename to apps/dav/lib/Files/BrowserErrorPagePlugin.php index 9f834cfbcb542..eccae8afdd533 100644 --- a/apps/dav/lib/Files/ErrorPagePlugin.php +++ b/apps/dav/lib/Files/BrowserErrorPagePlugin.php @@ -24,22 +24,17 @@ */ 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 ErrorPagePlugin extends ServerPlugin { - private ?Server $server = null; - - public function __construct( - private IRequest $request, - private IConfig $config, - ) { - } +class BrowserErrorPagePlugin extends ServerPlugin { + /** @var Server */ + private $server; /** * This initializes the plugin. @@ -48,12 +43,35 @@ public function __construct( * addPlugin is called. * * This method should set up the required event subscriptions. + * + * @param Server $server + * @return void */ - public function initialize(Server $server): void { + public function initialize(Server $server) { $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 \Throwable $ex + */ public function logException(\Throwable $ex): void { if ($ex instanceof Exception) { $httpCode = $ex->getHTTPCode(); @@ -64,7 +82,7 @@ public function logException(\Throwable $ex): void { } $this->server->httpResponse->addHeaders($headers); $this->server->httpResponse->setStatus($httpCode); - $body = $this->generateBody($ex, $httpCode); + $body = $this->generateBody($httpCode); $this->server->httpResponse->setBody($body); $csp = new ContentSecurityPolicy(); $this->server->httpResponse->addHeader('Content-Security-Policy', $csp->buildPolicy()); @@ -75,32 +93,18 @@ public function logException(\Throwable $ex): void { * @codeCoverageIgnore * @return bool|string */ - 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'); - } + public function generateBody(int $httpCode) { + $request = \OC::$server->getRequest(); - $debug = $this->config->getSystemValueBool('debug', false); + $templateName = 'exception'; + if ($httpCode === 403 || $httpCode === 404) { + $templateName = (string)$httpCode; + } - $content = new OC_Template('core', $templateName, $renderAs); + $content = new OC_Template('core', $templateName, 'guest'); $content->assign('title', $this->server->httpResponse->getStatusText()); - $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); + $content->assign('remoteAddr', $request->getRemoteAddress()); + $content->assign('requestID', $request->getId()); return $content->fetchPage(); } @@ -109,15 +113,6 @@ public function generateBody(\Throwable $ex, int $httpCode): mixed { */ public function sendResponse() { $this->server->sapi->sendResponse($this->server->httpResponse); - } - - 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; + exit(); } } diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 3b6a3a257898b..b717157b02a9c 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -36,6 +36,7 @@ */ namespace OCA\DAV; +use OC\Files\Filesystem; use OCA\DAV\AppInfo\PluginManager; use OCA\DAV\BulkUpload\BulkUploadPlugin; use OCA\DAV\CalDAV\BirthdayService; @@ -72,7 +73,7 @@ use OCA\DAV\DAV\ViewOnlyPlugin; use OCA\DAV\Events\SabrePluginAddEvent; use OCA\DAV\Events\SabrePluginAuthInitEvent; -use OCA\DAV\Files\ErrorPagePlugin; +use OCA\DAV\Files\BrowserErrorPagePlugin; use OCA\DAV\Files\LazySearchBackend; use OCA\DAV\Profiler\ProfilerPlugin; use OCA\DAV\Provisioning\Apple\AppleProvisioningPlugin; @@ -246,7 +247,9 @@ public function __construct(IRequest $request, string $baseUri) { $this->server->addPlugin(new FakeLockerPlugin()); } - $this->server->addPlugin(new ErrorPagePlugin($this->request, \OC::$server->getConfig())); + if (BrowserErrorPagePlugin::isBrowserRequest($request)) { + $this->server->addPlugin(new BrowserErrorPagePlugin()); + } $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 388d9df841383..cf4fcde251f4d 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 - {http://sabredav.org/ns}exception + {DAV:}valid-sync-token ignoreextras diff --git a/apps/dav/tests/unit/DAV/ErrorPagePluginTest.php b/apps/dav/tests/unit/DAV/BrowserErrorPagePluginTest.php similarity index 86% rename from apps/dav/tests/unit/DAV/ErrorPagePluginTest.php rename to apps/dav/tests/unit/DAV/BrowserErrorPagePluginTest.php index 3c87574e8d28f..b6ec05afd7875 100644 --- a/apps/dav/tests/unit/DAV/ErrorPagePluginTest.php +++ b/apps/dav/tests/unit/DAV/BrowserErrorPagePluginTest.php @@ -23,11 +23,11 @@ */ namespace OCA\DAV\Tests\unit\DAV; -use OCA\DAV\Files\ErrorPagePlugin; +use OCA\DAV\Files\BrowserErrorPagePlugin; use Sabre\DAV\Exception\NotFound; use Sabre\HTTP\Response; -class ErrorPagePluginTest extends \Test\TestCase { +class BrowserErrorPagePluginTest extends \Test\TestCase { /** * @dataProvider providesExceptions @@ -35,8 +35,8 @@ class ErrorPagePluginTest extends \Test\TestCase { * @param $exception */ public function test($expectedCode, $exception): void { - /** @var ErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */ - $plugin = $this->getMockBuilder(ErrorPagePlugin::class)->disableOriginalConstructor()->setMethods(['sendResponse', 'generateBody'])->getMock(); + /** @var BrowserErrorPagePlugin | \PHPUnit\Framework\MockObject\MockObject $plugin */ + $plugin = $this->getMockBuilder(BrowserErrorPagePlugin::class)->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 3e81a37cf671f..fffdd89d36748 100644 --- a/build/integration/features/caldav.feature +++ b/build/integration/features/caldav.feature @@ -3,7 +3,8 @@ 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 "Internal Server Error" + And The exception is "Sabre\DAV\Exception\NotFound" + And The error message is "Node with name 'MyCalendar' could not be found" Scenario: Accessing a not shared calendar of another user Given user "user0" exists @@ -11,7 +12,8 @@ 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 "Internal Server Error" + And The exception is "Sabre\DAV\Exception\NotFound" + And The error message is "Calendar with name 'MyCalendar' could not be found" Scenario: Accessing a not shared calendar of another user via the legacy endpoint Given user "user0" exists @@ -26,7 +28,8 @@ 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 "Internal Server Error" + And The exception is "Sabre\DAV\Exception\NotFound" + And The error message is "Node with name 'MyCalendar' could not be found" Scenario: Accessing a not existing calendar of another user via the legacy endpoint Given user "user0" exists @@ -39,7 +42,8 @@ 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 "Internal Server Error" + And The exception is "Sabre\DAV\Exception\NotFound" + And The error message is "Node with name 'MyCalendar' could not be found" Scenario: Creating a new calendar When "admin" creates a calendar named "MyCalendar" @@ -60,7 +64,8 @@ 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 "Internal Server Error" + And The exception is "Sabre\DAV\Exception\NotFound" + And The error message is "Node with name 'admin' could not be found" Scenario: Create calendar request for existing calendar of another user Given user "user0" exists @@ -68,4 +73,5 @@ 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 "Internal Server Error" + And The exception is "Sabre\DAV\Exception\NotFound" + And The error message is "Node with name 'admin' could not be found" diff --git a/build/integration/features/carddav.feature b/build/integration/features/carddav.feature index 15f1e95e73770..9c9df6ddd94be 100644 --- a/build/integration/features/carddav.feature +++ b/build/integration/features/carddav.feature @@ -2,13 +2,15 @@ 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 "Internal Server Error" + And The CardDAV exception is "Sabre\DAV\Exception\NotFound" + And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found" 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 "Internal Server Error" + And The CardDAV exception is "Sabre\DAV\Exception\NotFound" + And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found" Scenario: Accessing a not existing addressbook of another user via legacy endpoint Given user "user0" exists @@ -26,7 +28,8 @@ 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 "Internal Server Error" + And The CardDAV exception is "Sabre\DAV\Exception\NotFound" + And The CardDAV error message is "Addressbook with name 'MyAddressbook' could not be found" Scenario: Creating a new addressbook When "admin" creates an addressbook named "MyAddressbook" with statuscode "201" @@ -64,11 +67,13 @@ 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 "Internal Server Error" + And The CardDAV exception is "Sabre\DAV\Exception\NotFound" + And The CardDAV error message is "File not found: admin in 'addressbooks'" 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 "Internal Server Error" + And The CardDAV exception is "Sabre\DAV\Exception\NotFound" + And The CardDAV error message is "File not found: admin in 'addressbooks'" diff --git a/remote.php b/remote.php index 03575627dc504..87629ef2971b5 100644 --- a/remote.php +++ b/remote.php @@ -52,9 +52,10 @@ class RemoteException extends Exception { function handleException($e) { try { $request = \OC::$server->getRequest(); + $isError = $e instanceof Error; // in case the request content type is text/xml - we assume it's a WebDAV request $isXmlContentType = strpos($request->getHeader('Content-Type'), 'text/xml'); - if ($isXmlContentType === 0) { + if ($isError === false && $isXmlContentType === 0) { // fire up a simple server to properly process the exception $server = new Server(); if (!($e instanceof RemoteException)) {