fix(dav): Always respond custom error page on exceptions

Signed-off-by: Louis Chemineau <louis@chmn.me>

3 files changed, 48 insertions, 46 deletions

diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php
index fc35d78a2f7..2d804aad842 100644
--- a/apps/dav/lib/Connector/Sabre/ServerFactory.php
+++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php
@@ -10,7 +10,7 @@ namespace OCA\DAV\Connector\Sabre;
 use OCA\DAV\AppInfo\PluginManager;
 use OCA\DAV\CalDAV\DefaultCalendarValidator;
 use OCA\DAV\DAV\ViewOnlyPlugin;
-use OCA\DAV\Files\BrowserErrorPagePlugin;
+use OCA\DAV\Files\ErrorPagePlugin;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Folder;
 use OCP\Files\IFilenameValidator;
@@ -98,9 +98,7 @@ class ServerFactory {
 			$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
index 46598db2040..ddf04d0f763 100644
--- a/apps/dav/lib/Files/BrowserErrorPagePlugin.php
+++ b/apps/dav/lib/Files/ErrorPagePlugin.php
@@ -7,17 +7,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.
@@ -26,35 +31,12 @@ 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 \Throwable $ex
-	 */
 	public function logException(\Throwable $ex): void {
 		if ($ex instanceof Exception) {
 			$httpCode = $ex->getHTTPCode();
@@ -65,7 +47,7 @@ class BrowserErrorPagePlugin extends ServerPlugin {
 		}
 		$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());
@@ -76,18 +58,32 @@ class BrowserErrorPagePlugin extends ServerPlugin {
 	 * @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();
 	}
 
@@ -98,4 +94,14 @@ class BrowserErrorPagePlugin extends ServerPlugin {
 		$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 449b8ef0ceb..55c8a5afec1 100644
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@@ -43,7 +43,7 @@ use OCA\DAV\DAV\PublicAuth;
 use OCA\DAV\DAV\ViewOnlyPlugin;
 use OCA\DAV\Events\SabrePluginAddEvent;
 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;
@@ -221,9 +221,7 @@ class Server {
 			$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));