Skip to content

Commit

Permalink
Merge pull request #48304 from nextcloud/backport/48297/stable25
Browse files Browse the repository at this point in the history
[stable25] fix(dav): Always respond custom error page on exceptions
  • Loading branch information
sorbaugh authored Oct 1, 2024
2 parents 5e15d2c + fc6ab73 commit 188d747
Show file tree
Hide file tree
Showing 15 changed files with 249 additions and 169 deletions.
2 changes: 1 addition & 1 deletion apps/dav/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,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',
Expand Down
2 changes: 1 addition & 1 deletion apps/dav/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,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',
Expand Down
23 changes: 23 additions & 0 deletions apps/dav/lib/Connector/Sabre/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
}
}
}
}
6 changes: 2 additions & 4 deletions apps/dav/lib/Connector/Sabre/ServerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
118 changes: 0 additions & 118 deletions apps/dav/lib/Files/BrowserErrorPagePlugin.php

This file was deleted.

127 changes: 127 additions & 0 deletions apps/dav/lib/Files/ErrorPagePlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
<?php
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
* @author Arthur Schiwon <[email protected]>
* @author Christoph Wurst <[email protected]>
* @author Lukas Reschke <[email protected]>
* @author Thomas Müller <[email protected]>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\DAV\Files;

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;
private IRequest $request;
private IConfig $config;

public function __construct(
IRequest $request,
IConfig $config
) {
$this->request = $request;
$this->config = $config;
}

/**
* This initializes the plugin.
*
* This function is called by Sabre\DAV\Server, after
* addPlugin is called.
*
* This method should set up the required event subscriptions.
*/
public function initialize(Server $server): void {
$this->server = $server;
$server->on('exception', [$this, 'logException'], 1000);
}

public function logException(\Throwable $ex): void {
if ($ex instanceof Exception) {
$httpCode = $ex->getHTTPCode();
$headers = $ex->getHTTPHeaders($this->server);
} else {
$httpCode = 500;
$headers = [];
}
$this->server->httpResponse->addHeaders($headers);
$this->server->httpResponse->setStatus($httpCode);
$body = $this->generateBody($ex, $httpCode);
$this->server->httpResponse->setBody($body);
$csp = new ContentSecurityPolicy();
$this->server->httpResponse->addHeader('Content-Security-Policy', $csp->buildPolicy());
$this->sendResponse();
}

/**
* @codeCoverageIgnore
* @return string
*/
public function generateBody(\Throwable $ex, int $httpCode) {
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');
}

$debug = $this->config->getSystemValueBool('debug', false);

$content = new OC_Template('core', $templateName, $renderAs);
$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);
return $content->fetchPage();
}

/**
* @codeCoverageIgnore
*/
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;
}
}
6 changes: 2 additions & 4 deletions apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,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;
Expand Down Expand Up @@ -236,9 +236,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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2712,7 +2712,7 @@
<callback>prepostcondition</callback>
<arg>
<name>error</name>
<value>{DAV:}valid-sync-token</value>
<value>{http://sabredav.org/ns}exception</value>
</arg>
<arg>
<name>ignoreextras</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,20 @@
*/
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
* @param $expectedCode
* @param $exception
*/
public function test($expectedCode, $exception) {
/** @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 */
Expand Down
Loading

0 comments on commit 188d747

Please sign in to comment.