From 0f3e36fdfd1c57f5212c495d59b0e5964df8a4ac Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20M=C3=BCller?= Date: Wed, 11 Mar 2015 11:53:31 +0100 Subject: [PATCH] Adding a more meaningful message for sabre dav exception - fixes #14516 --- apps/files/appinfo/remote.php | 2 +- apps/files_sharing/publicwebdav.php | 2 +- .../connector/sabre/exceptionloggerplugin.php | 36 ++++++++-- .../connector/sabre/exceptionloggerplugin.php | 71 +++++++++++++++++++ tests/lib/connector/sabre/file.php | 3 - 5 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 tests/lib/connector/sabre/exceptionloggerplugin.php diff --git a/apps/files/appinfo/remote.php b/apps/files/appinfo/remote.php index f631e47b5f6..e260f85fdb0 100644 --- a/apps/files/appinfo/remote.php +++ b/apps/files/appinfo/remote.php @@ -40,7 +40,7 @@ $server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend, $defaults->getName() $server->addPlugin(new \OC\Connector\Sabre\DummyGetResponsePlugin()); $server->addPlugin(new \OC\Connector\Sabre\FilesPlugin($objectTree)); $server->addPlugin(new \OC\Connector\Sabre\MaintenancePlugin()); -$server->addPlugin(new \OC\Connector\Sabre\ExceptionLoggerPlugin('webdav')); +$server->addPlugin(new \OC\Connector\Sabre\ExceptionLoggerPlugin('webdav', \OC::$server->getLogger())); // wait with registering these until auth is handled and the filesystem is setup $server->on('beforeMethod', function () use ($server, $objectTree) { diff --git a/apps/files_sharing/publicwebdav.php b/apps/files_sharing/publicwebdav.php index 150f05a862a..c7ee950532e 100644 --- a/apps/files_sharing/publicwebdav.php +++ b/apps/files_sharing/publicwebdav.php @@ -33,7 +33,7 @@ $server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend, $defaults->getName() $server->addPlugin(new \Sabre\DAV\Browser\Plugin(false)); // Show something in the Browser, but no upload $server->addPlugin(new \OC\Connector\Sabre\FilesPlugin($objectTree)); $server->addPlugin(new \OC\Connector\Sabre\MaintenancePlugin()); -$server->addPlugin(new \OC\Connector\Sabre\ExceptionLoggerPlugin('webdav')); +$server->addPlugin(new \OC\Connector\Sabre\ExceptionLoggerPlugin('webdav', \OC::$server->getLogger())); // wait with registering these until auth is handled and the filesystem is setup $server->on('beforeMethod', function () use ($server, $objectTree, $authBackend) { diff --git a/lib/private/connector/sabre/exceptionloggerplugin.php b/lib/private/connector/sabre/exceptionloggerplugin.php index 2bd43e56f55..a0dc6e7c182 100644 --- a/lib/private/connector/sabre/exceptionloggerplugin.php +++ b/lib/private/connector/sabre/exceptionloggerplugin.php @@ -10,6 +10,10 @@ namespace OC\Connector\Sabre; +use OCP\ILogger; +use Sabre\DAV\Exception; +use Sabre\HTTP\Response; + class ExceptionLoggerPlugin extends \Sabre\DAV\ServerPlugin { private $nonFatalExceptions = array( 'Sabre\DAV\Exception\NotAuthenticated' => true, @@ -22,13 +26,19 @@ class ExceptionLoggerPlugin extends \Sabre\DAV\ServerPlugin { 'Sabre\DAV\Exception\PreconditionFailed' => true, ); + /** @var string */ private $appName; + /** @var ILogger */ + private $logger; + /** * @param string $loggerAppName app name to use when logging + * @param ILogger $logger */ - public function __construct($loggerAppName = 'webdav') { + public function __construct($loggerAppName, $logger) { $this->appName = $loggerAppName; + $this->logger = $logger; } /** @@ -50,14 +60,30 @@ class ExceptionLoggerPlugin extends \Sabre\DAV\ServerPlugin { /** * Log exception * - * @internal param Exception $e exception */ - public function logException($e) { - $exceptionClass = get_class($e); + public function logException(\Exception $ex) { + $exceptionClass = get_class($ex); $level = \OCP\Util::FATAL; if (isset($this->nonFatalExceptions[$exceptionClass])) { $level = \OCP\Util::DEBUG; } - \OCP\Util::logException($this->appName, $e, $level); + + $message = $ex->getMessage(); + if ($ex instanceof Exception) { + if (empty($message)) { + $response = new Response($ex->getHTTPCode()); + $message = $response->getStatusText(); + } + $message = "HTTP/1.1 {$ex->getHTTPCode()} $message"; + } + + $exception = [ + 'Message' => $message, + 'Code' => $ex->getCode(), + 'Trace' => $ex->getTraceAsString(), + 'File' => $ex->getFile(), + 'Line' => $ex->getLine(), + ]; + $this->logger->log($level, 'Exception: ' . json_encode($exception), ['app' => $this->appName]); } } diff --git a/tests/lib/connector/sabre/exceptionloggerplugin.php b/tests/lib/connector/sabre/exceptionloggerplugin.php new file mode 100644 index 00000000000..0662ba029d9 --- /dev/null +++ b/tests/lib/connector/sabre/exceptionloggerplugin.php @@ -0,0 +1,71 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace Test\Connector\Sabre; + +use OC\Connector\Sabre\Exception\InvalidPath; +use OC\Connector\Sabre\ExceptionLoggerPlugin as PluginToTest; +use OC\Log; +use OCP\ILogger; +use PHPUnit_Framework_MockObject_MockObject; +use Sabre\DAV\Exception\NotFound; +use Sabre\DAV\Server; +use Test\TestCase; + +class TestLogger extends Log { + public $message; + public $level; + + public function __construct($logger = null) { + //disable original constructor + } + + public function log($level, $message, array $context = array()) { + $this->level = $level; + $this->message = $message; + } +} + +class ExceptionLoggerPlugin extends TestCase { + + /** @var Server */ + private $server; + + /** @var PluginToTest */ + private $plugin; + + /** @var TestLogger | PHPUnit_Framework_MockObject_MockObject */ + private $logger; + + private function init() { + $this->server = new Server(); + $this->logger = new TestLogger(); + $this->plugin = new PluginToTest('unit-test', $this->logger); + $this->plugin->initialize($this->server); + } + + /** + * @dataProvider providesExceptions + */ + public function testLogging($expectedLogLevel, $expectedMessage, $exception) { + $this->init(); + $this->plugin->logException($exception); + + $this->assertEquals($expectedLogLevel, $this->logger->level); + $this->assertStringStartsWith('Exception: {"Message":"' . $expectedMessage, $this->logger->message); + } + + public function providesExceptions() { + return [ + [0, 'HTTP\/1.1 404 Not Found', new NotFound()], + [4, 'HTTP\/1.1 400 This path leads to nowhere', new InvalidPath('This path leads to nowhere')] + ]; + } + +} diff --git a/tests/lib/connector/sabre/file.php b/tests/lib/connector/sabre/file.php index f2812e390ac..74e289c1751 100644 --- a/tests/lib/connector/sabre/file.php +++ b/tests/lib/connector/sabre/file.php @@ -8,9 +8,6 @@ namespace Test\Connector\Sabre; - -use OC_Connector_Sabre_File; - class File extends \Test\TestCase { /** -- 2.39.5