Adding a more meaningful message for sabre dav exception - fixes #14516

5 files changed, 104 insertions, 10 deletions

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 @@
+<?php
+
+/**
+ * Copyright (c) 2015 Thomas Müller <deepdiver@owncloud.com>
+ * 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 {
 
 	/**