diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 39b91f8dc9..863199dfb9 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -278,6 +278,8 @@ return array( 'OCP\\Share_Backend' => $baseDir . '/lib/public/Share_Backend.php', 'OCP\\Share_Backend_Collection' => $baseDir . '/lib/public/Share_Backend_Collection.php', 'OCP\\Share_Backend_File_Dependent' => $baseDir . '/lib/public/Share_Backend_File_Dependent.php', + 'OCP\\Support\\CrashReport\\IRegistry' => $baseDir . '/lib/public/Support/CrashReport/IRegistry.php', + 'OCP\\Support\\CrashReport\\IReporter' => $baseDir . '/lib/public/Support/CrashReport/IReporter.php', 'OCP\\SystemTag\\ISystemTag' => $baseDir . '/lib/public/SystemTag/ISystemTag.php', 'OCP\\SystemTag\\ISystemTagManager' => $baseDir . '/lib/public/SystemTag/ISystemTagManager.php', 'OCP\\SystemTag\\ISystemTagManagerFactory' => $baseDir . '/lib/public/SystemTag/ISystemTagManagerFactory.php', @@ -899,6 +901,7 @@ return array( 'OC\\Share\\Share' => $baseDir . '/lib/private/Share/Share.php', 'OC\\Streamer' => $baseDir . '/lib/private/Streamer.php', 'OC\\SubAdmin' => $baseDir . '/lib/private/SubAdmin.php', + 'OC\\Support\\CrashReport\\Registry' => $baseDir . '/lib/private/Support/CrashReport/Registry.php', 'OC\\SystemConfig' => $baseDir . '/lib/private/SystemConfig.php', 'OC\\SystemTag\\ManagerFactory' => $baseDir . '/lib/private/SystemTag/ManagerFactory.php', 'OC\\SystemTag\\SystemTag' => $baseDir . '/lib/private/SystemTag/SystemTag.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index e5a9808670..e52ee329e2 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -308,6 +308,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\Share_Backend' => __DIR__ . '/../../..' . '/lib/public/Share_Backend.php', 'OCP\\Share_Backend_Collection' => __DIR__ . '/../../..' . '/lib/public/Share_Backend_Collection.php', 'OCP\\Share_Backend_File_Dependent' => __DIR__ . '/../../..' . '/lib/public/Share_Backend_File_Dependent.php', + 'OCP\\Support\\CrashReport\\IRegistry' => __DIR__ . '/../../..' . '/lib/public/Support/CrashReport/IRegistry.php', + 'OCP\\Support\\CrashReport\\IReporter' => __DIR__ . '/../../..' . '/lib/public/Support/CrashReport/IReporter.php', 'OCP\\SystemTag\\ISystemTag' => __DIR__ . '/../../..' . '/lib/public/SystemTag/ISystemTag.php', 'OCP\\SystemTag\\ISystemTagManager' => __DIR__ . '/../../..' . '/lib/public/SystemTag/ISystemTagManager.php', 'OCP\\SystemTag\\ISystemTagManagerFactory' => __DIR__ . '/../../..' . '/lib/public/SystemTag/ISystemTagManagerFactory.php', @@ -929,6 +931,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Share\\Share' => __DIR__ . '/../../..' . '/lib/private/Share/Share.php', 'OC\\Streamer' => __DIR__ . '/../../..' . '/lib/private/Streamer.php', 'OC\\SubAdmin' => __DIR__ . '/../../..' . '/lib/private/SubAdmin.php', + 'OC\\Support\\CrashReport\\Registry' => __DIR__ . '/../../..' . '/lib/private/Support/CrashReport/Registry.php', 'OC\\SystemConfig' => __DIR__ . '/../../..' . '/lib/private/SystemConfig.php', 'OC\\SystemTag\\ManagerFactory' => __DIR__ . '/../../..' . '/lib/private/SystemTag/ManagerFactory.php', 'OC\\SystemTag\\SystemTag' => __DIR__ . '/../../..' . '/lib/private/SystemTag/SystemTag.php', diff --git a/lib/private/Log.php b/lib/private/Log.php index 0d1f7a0126..6b97a3a028 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -36,7 +36,8 @@ namespace OC; use InterfaSys\LogNormalizer\Normalizer; -use \OCP\ILogger; +use OCP\ILogger; +use OCP\Support\CrashReport\IRegistry; use OCP\Util; /** @@ -63,6 +64,9 @@ class Log implements ILogger { /** @var Normalizer */ private $normalizer; + /** @var IRegistry */ + private $crashReporters; + protected $methodsWithSensitiveParameters = [ // Session/User 'completeLogin', @@ -110,9 +114,10 @@ class Log implements ILogger { /** * @param string $logger The logger that should be used * @param SystemConfig $config the system config object - * @param null $normalizer + * @param Normalizer|null $normalizer + * @param IRegistry|null $registry */ - public function __construct($logger = null, SystemConfig $config = null, $normalizer = null) { + public function __construct($logger = null, SystemConfig $config = null, $normalizer = null, IRegistry $registry = null) { // FIXME: Add this for backwards compatibility, should be fixed at some point probably if($config === null) { $config = \OC::$server->getSystemConfig(); @@ -133,7 +138,7 @@ class Log implements ILogger { } else { $this->normalizer = $normalizer; } - + $this->crashReporters = $registry; } /** @@ -346,6 +351,9 @@ class Log implements ILogger { $msg = isset($context['message']) ? $context['message'] : 'Exception'; $msg .= ': ' . json_encode($data); $this->log($level, $msg, $context); + if (!is_null($this->crashReporters)) { + $this->crashReporters->delegateReport($exception); + } } /** diff --git a/lib/private/Server.php b/lib/private/Server.php index af18b1a3a1..1e3ac3de27 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -516,12 +516,16 @@ class Server extends ServerContainer implements IServerContainer { }); $this->registerAlias('AvatarManager', \OCP\IAvatarManager::class); + $this->registerAlias(\OCP\Support\CrashReport\IRegistry::class, \OC\Support\CrashReport\Registry::class); + $this->registerService(\OCP\ILogger::class, function (Server $c) { $logType = $c->query('AllConfig')->getSystemValue('log_type', 'file'); $logger = Log::getLogClass($logType); call_user_func(array($logger, 'init')); + $config = $this->getSystemConfig(); + $registry = $c->query(\OCP\Support\CrashReport\IRegistry::class); - return new Log($logger); + return new Log($logger, $config, null, $registry); }); $this->registerAlias('Logger', \OCP\ILogger::class); diff --git a/lib/private/Support/CrashReport/Registry.php b/lib/private/Support/CrashReport/Registry.php new file mode 100644 index 0000000000..bdf18b492f --- /dev/null +++ b/lib/private/Support/CrashReport/Registry.php @@ -0,0 +1,55 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace OC\Support\CrashReport; + +use Exception; +use OCP\Support\CrashReport\IRegistry; +use OCP\Support\CrashReport\IReporter; +use Throwable; + +class Registry implements IRegistry { + + /** @var array */ + private $reporters = []; + + /** + * Register a reporter instance + * + * @param IReporter $reporter + */ + public function register(IReporter $reporter) { + $this->reporters[] = $reporter; + } + + /** + * Delegate crash reporting to all registered reporters + * + * @param Exception|Throwable $exception + */ + public function delegateReport($exception) { + foreach ($this->reporters as $reporter) { + $reporter->report($exception); + } + } + +} diff --git a/lib/public/Support/CrashReport/IRegistry.php b/lib/public/Support/CrashReport/IRegistry.php new file mode 100644 index 0000000000..66c527092b --- /dev/null +++ b/lib/public/Support/CrashReport/IRegistry.php @@ -0,0 +1,48 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace OCP\Support\CrashReport; + +use Exception; +use Throwable; + +/** + * @since 13.0.0 + */ +interface IRegistry { + + /** + * Register a reporter instance + * + * @since 13.0.0 + * @param IReporter $reporter + */ + public function register(IReporter $reporter); + + /** + * Delegate crash reporting to all registered reporters + * + * @since 13.0.0 + * @param Exception|Throwable $exception + */ + public function delegateReport($exception); +} diff --git a/lib/public/Support/CrashReport/IReporter.php b/lib/public/Support/CrashReport/IReporter.php new file mode 100644 index 0000000000..03c4f47e3b --- /dev/null +++ b/lib/public/Support/CrashReport/IReporter.php @@ -0,0 +1,40 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace OCP\Support\CrashReport; + +use Exception; +use Throwable; + +/** + * @since 13.0.0 + */ +interface IReporter { + + /** + * Report an (unhandled) exception + * + * @since 13.0.0 + * @param Exception|Throwable $exception + */ + public function report($exception); +} diff --git a/tests/lib/LoggerTest.php b/tests/lib/LoggerTest.php index 3a30bbd1d3..76f33849de 100644 --- a/tests/lib/LoggerTest.php +++ b/tests/lib/LoggerTest.php @@ -11,21 +11,26 @@ namespace Test; use OC\Log; class LoggerTest extends TestCase { - /** - * @var \OCP\ILogger - */ + + /** @var \OC\SystemConfig|\PHPUnit_Framework_MockObject_MockObject */ + private $config; + + /** @var \OCP\Support\CrashReport\IRegistry|\PHPUnit_Framework_MockObject_MockObject */ + private $registry; + + /** @var \OCP\ILogger */ private $logger; + + /** @var array */ static private $logs = array(); protected function setUp() { parent::setUp(); self::$logs = array(); - $this->config = $this->getMockBuilder( - '\OC\SystemConfig') - ->disableOriginalConstructor() - ->getMock(); - $this->logger = new Log('Test\LoggerTest', $this->config); + $this->config = $this->createMock(\OC\SystemConfig::class); + $this->registry = $this->createMock(\OCP\Support\CrashReport\IRegistry::class); + $this->logger = new Log('Test\LoggerTest', $this->config, null, $this->registry); } public function testInterpolation() { @@ -83,6 +88,10 @@ class LoggerTest extends TestCase { */ public function testDetectlogin($user, $password) { $e = new \Exception('test'); + $this->registry->expects($this->once()) + ->method('delegateReport') + ->with($e); + $this->logger->logException($e); $logLines = $this->getLogs(); @@ -98,9 +107,13 @@ class LoggerTest extends TestCase { */ public function testDetectcheckPassword($user, $password) { $e = new \Exception('test'); - $this->logger->logException($e); - $logLines = $this->getLogs(); + $this->registry->expects($this->once()) + ->method('delegateReport') + ->with($e); + $this->logger->logException($e); + + $logLines = $this->getLogs(); foreach($logLines as $logLine) { $this->assertNotContains($user, $logLine); $this->assertNotContains($password, $logLine); @@ -113,9 +126,13 @@ class LoggerTest extends TestCase { */ public function testDetectvalidateUserPass($user, $password) { $e = new \Exception('test'); - $this->logger->logException($e); - $logLines = $this->getLogs(); + $this->registry->expects($this->once()) + ->method('delegateReport') + ->with($e); + $this->logger->logException($e); + + $logLines = $this->getLogs(); foreach($logLines as $logLine) { $this->assertNotContains($user, $logLine); $this->assertNotContains($password, $logLine); @@ -128,9 +145,13 @@ class LoggerTest extends TestCase { */ public function testDetecttryLogin($user, $password) { $e = new \Exception('test'); - $this->logger->logException($e); - $logLines = $this->getLogs(); + $this->registry->expects($this->once()) + ->method('delegateReport') + ->with($e); + $this->logger->logException($e); + + $logLines = $this->getLogs(); foreach($logLines as $logLine) { $this->assertNotContains($user, $logLine); $this->assertNotContains($password, $logLine); @@ -145,14 +166,16 @@ class LoggerTest extends TestCase { $a = function($user, $password) { throw new \Exception('test'); }; + $this->registry->expects($this->once()) + ->method('delegateReport'); try { $a($user, $password); } catch (\Exception $e) { $this->logger->logException($e); } - $logLines = $this->getLogs(); + $logLines = $this->getLogs(); foreach($logLines as $logLine) { $log = explode('\n', $logLine); unset($log[1]); // Remove `testDetectclosure(` because we are not testing this here, but the closure on stack trace 0 diff --git a/tests/lib/Support/CrashReport/RegistryTest.php b/tests/lib/Support/CrashReport/RegistryTest.php new file mode 100644 index 0000000000..b73bf2737e --- /dev/null +++ b/tests/lib/Support/CrashReport/RegistryTest.php @@ -0,0 +1,68 @@ + + * + * @author 2017 Christoph Wurst + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace Test\Support\CrashReport; + +use Exception; +use OC\Support\CrashReport\Registry; +use OCP\Support\CrashReport\IReporter; +use Test\TestCase; + +class RegistryTest extends TestCase { + + /** @var Registry */ + private $registry; + + protected function setUp() { + parent::setUp(); + + $this->registry = new Registry(); + } + + /** + * Doesn't assert anything, just checks whether anything "explodes" + */ + public function testDelegateToNone() { + $exception = new Exception('test'); + + $this->registry->delegateReport($exception); + } + + public function testDelegateToAll() { + $reporter1 = $this->createMock(IReporter::class); + $reporter2 = $this->createMock(IReporter::class); + $exception = new Exception('test'); + $reporter1->expects($this->once()) + ->method('report') + ->with($exception); + $reporter2->expects($this->once()) + ->method('report') + ->with($exception); + + $this->registry->register($reporter1); + $this->registry->register($reporter2); + $this->registry->delegateReport($exception); + } + +}