[FOLLOWUP][BUGFIX] Handle exceptions in Logging API 96/37896/2
authorSteffen Müller <typo3@t3node.com>
Sun, 8 Mar 2015 14:43:28 +0000 (15:43 +0100)
committerMarkus Klein <klein.t3@reelworx.at>
Mon, 16 Mar 2015 22:30:38 +0000 (23:30 +0100)
Do not run exceptions through json_encode() but convert them to strings
first.

Resolves: #65577
Releases: master, 6.2
Change-Id: I8c893f64942f8ec18aed1cef5a276345b3f568fe
Reviewed-on: http://review.typo3.org/37896
Reviewed-by: Markus Klein <klein.t3@reelworx.at>
Tested-by: Markus Klein <klein.t3@reelworx.at>
typo3/sysext/core/Classes/Log/LogRecord.php
typo3/sysext/core/Classes/Log/Writer/DatabaseWriter.php
typo3/sysext/core/Classes/Log/Writer/FileWriter.php
typo3/sysext/core/Classes/Log/Writer/NullWriter.php
typo3/sysext/core/Classes/Log/Writer/PhpErrorLogWriter.php
typo3/sysext/core/Classes/Log/Writer/SyslogWriter.php
typo3/sysext/core/Tests/Unit/Log/Writer/DatabaseWriterTest.php

index dcf85c3..e3b87e6 100644 (file)
@@ -262,7 +262,15 @@ class LogRecord implements \ArrayAccess {
                        }
                        $data = '- ' . json_encode($this->data);
                }
-               $logRecordString = sprintf('%s [%s] request="%s" component="%s": %s %s', $timestamp, $levelName, $this->requestId, $this->component, $this->message, $data);
+               $logRecordString = sprintf(
+                       '%s [%s] request="%s" component="%s": %s %s',
+                       $timestamp,
+                       $levelName,
+                       $this->requestId,
+                       $this->component,
+                       $this->message,
+                       $data
+               );
                return $logRecordString;
        }
 
index 0ef7b73..907bee9 100644 (file)
@@ -13,6 +13,9 @@ namespace TYPO3\CMS\Core\Log\Writer;
  *
  * The TYPO3 project - inspiring people to share!
  */
+
+use TYPO3\CMS\Core\Log\LogRecord;
+
 /**
  * Log writer that writes the log records into a database table.
  *
@@ -50,23 +53,42 @@ class DatabaseWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
        /**
         * Writes the log record
         *
-        * @param \TYPO3\CMS\Core\Log\LogRecord $record Log record
+        * @param LogRecord $record Log record
         * @return \TYPO3\CMS\Core\Log\Writer\WriterInterface $this
         * @throws \RuntimeException
         */
-       public function writeLog(\TYPO3\CMS\Core\Log\LogRecord $record) {
-               $data = array(
-                       'request_id' => $record['requestId'],
-                       'time_micro' => $record['created'],
-                       'component' => $record['component'],
-                       'level' => $record['level'],
-                       'message' => $record['message'],
-                       'data' => !empty($record['data']) ? json_encode($record['data']) : ''
+       public function writeLog(LogRecord $record) {
+               $data = '';
+               $recordData = $record->getData();
+               if (!empty($recordData)) {
+                       // According to PSR3 the exception-key may hold an \Exception
+                       // Since json_encode() does not encode an exception, we run the _toString() here
+                       if (isset($recordData['exception']) && $recordData['exception'] instanceof \Exception) {
+                               $recordData['exception'] = (string)$recordData['exception'];
+                       }
+                       $data = '- ' . json_encode($recordData);
+               }
+
+               $fieldValues = array(
+                       'request_id' => $record->getRequestId(),
+                       'time_micro' => $record->getCreated(),
+                       'component' => $record->getComponent(),
+                       'level' => $record->getLevel(),
+                       'message' => $record->getMessage(),
+                       'data' => $data
                );
-               if (FALSE === $GLOBALS['TYPO3_DB']->exec_INSERTquery($this->logTable, $data)) {
+
+               if (FALSE === $this->getDatabaseConnection()->exec_INSERTquery($this->logTable, $fieldValues)) {
                        throw new \RuntimeException('Could not write log record to database', 1345036334);
                }
                return $this;
        }
 
+       /**
+        * @return \TYPO3\CMS\Core\Database\DatabaseConnection
+        */
+       protected function getDatabaseConnection() {
+               return $GLOBALS['TYPO3_DB'];
+       }
+
 }
index 2ece53b..08fbc23 100644 (file)
@@ -14,7 +14,9 @@ namespace TYPO3\CMS\Core\Log\Writer;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Log\LogLevel;
 use TYPO3\CMS\Core\Log\LogRecord;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 /**
  * Log writer that writes the log records into a file.
@@ -23,7 +25,7 @@ use TYPO3\CMS\Core\Log\LogRecord;
  * @author Steffen Müller <typo3@t3node.com>
  * @author Ingo Renner <ingo@typo3.org>
  */
-class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
+class FileWriter extends AbstractWriter {
 
        /**
         * Log file path, relative to PATH_site
@@ -82,10 +84,10 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
 
                // Skip handling if logFile is a stream resource. This is used by unit tests with vfs:// directories
                if (FALSE === strpos($logFile, '://')) {
-                       if (!\TYPO3\CMS\Core\Utility\GeneralUtility::isAllowedAbsPath((PATH_site . $logFile))) {
+                       if (!GeneralUtility::isAllowedAbsPath((PATH_site . $logFile))) {
                                throw new \InvalidArgumentException('Log file path "' . $logFile . '" is not valid!', 1326411176);
                        }
-                       $logFile = \TYPO3\CMS\Core\Utility\GeneralUtility::getFileAbsFileName($logFile);
+                       $logFile = GeneralUtility::getFileAbsFileName($logFile);
                }
                $this->logFile = $logFile;
                $this->openLogFile();
@@ -110,7 +112,30 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
         * @throws \RuntimeException
         */
        public function writeLog(LogRecord $record) {
-               if (FALSE === fwrite(self::$logFileHandles[$this->logFile], $record . LF)) {
+               $timestamp = date('r', (int)$record->getCreated());
+               $levelName = LogLevel::getName($record->getLevel());
+               $data = '';
+               $recordData = $record->getData();
+               if (!empty($recordData)) {
+                       // According to PSR3 the exception-key may hold an \Exception
+                       // Since json_encode() does not encode an exception, we run the _toString() here
+                       if (isset($recordData['exception']) && $recordData['exception'] instanceof \Exception) {
+                               $recordData['exception'] = (string)$recordData['exception'];
+                       }
+                       $data = '- ' . json_encode($recordData);
+               }
+
+               $message = sprintf(
+                       '%s [%s] request="%s" component="%s": %s %s',
+                       $timestamp,
+                       $levelName,
+                       $record->getRequestId(),
+                       $record->getComponent(),
+                       $record->getMessage(),
+                       $data
+               );
+
+               if (FALSE === fwrite(self::$logFileHandles[$this->logFile], $message . LF)) {
                        throw new \RuntimeException('Could not write log record to log file', 1345036335);
                }
 
@@ -159,12 +184,12 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
                }
                $logFileDirectory = dirname($this->logFile);
                if (!@is_dir($logFileDirectory)) {
-                       \TYPO3\CMS\Core\Utility\GeneralUtility::mkdir_deep($logFileDirectory);
+                       GeneralUtility::mkdir_deep($logFileDirectory);
                        // only create .htaccess, if we created the directory on our own
                        $this->createHtaccessFile($logFileDirectory . '/.htaccess');
                }
                // create the log file
-               \TYPO3\CMS\Core\Utility\GeneralUtility::writeFile($this->logFile, '');
+               GeneralUtility::writeFile($this->logFile, '');
        }
 
        /**
@@ -176,7 +201,7 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
        protected function createHtaccessFile($htaccessFile) {
                // write .htaccess file to protect the log file
                if (!empty($GLOBALS['TYPO3_CONF_VARS']['SYS']['generateApacheHtaccess']) && !file_exists($htaccessFile)) {
-                       \TYPO3\CMS\Core\Utility\GeneralUtility::writeFile($htaccessFile, 'Deny From All');
+                       GeneralUtility::writeFile($htaccessFile, 'Deny From All');
                }
        }
 
index b5bef99..4ca3910 100644 (file)
@@ -13,20 +13,23 @@ namespace TYPO3\CMS\Core\Log\Writer;
  *
  * The TYPO3 project - inspiring people to share!
  */
+
+use TYPO3\CMS\Core\Log\LogRecord;
+
 /**
  * Null writer - just forgets about everything
  *
  * @author Ingo Renner <ingo@typo3.org>
  */
-class NullWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
+class NullWriter extends AbstractWriter {
 
        /**
         * Writes the log record
         *
-        * @param \TYPO3\CMS\Core\Log\LogRecord $record Log record
+        * @param LogRecord $record Log record
         * @return \TYPO3\CMS\Core\Log\Writer\WriterInterface $this
         */
-       public function writeLog(\TYPO3\CMS\Core\Log\LogRecord $record) {
+       public function writeLog(LogRecord $record) {
                // do nothing
                return $this;
        }
index 7cdba9f..163bc92 100644 (file)
@@ -13,26 +13,45 @@ namespace TYPO3\CMS\Core\Log\Writer;
  *
  * The TYPO3 project - inspiring people to share!
  */
+
+use TYPO3\CMS\Core\Log\LogRecord;
+use TYPO3\CMS\Core\Log\LogLevel;
+
 /**
  * Log writer that writes the log records into PHP error log.
  *
  * @author Steffen Gebert <steffen.gebert@typo3.org>
  * @author Steffen Müller <typo3@t3node.com>
  */
-class PhpErrorLogWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
+class PhpErrorLogWriter extends AbstractWriter {
 
        /**
         * Writes the log record
         *
-        * @param \TYPO3\CMS\Core\Log\LogRecord $record Log record
+        * @param LogRecord $record Log record
         * @return \TYPO3\CMS\Core\Log\Writer\WriterInterface $this
         * @throws \RuntimeException
         */
-       public function writeLog(\TYPO3\CMS\Core\Log\LogRecord $record) {
-               $levelName = \TYPO3\CMS\Core\Log\LogLevel::getName($record->getLevel());
-               $data = $record->getData();
-               $data = !empty($data) ? '- ' . json_encode($data) : '';
-               $message = sprintf('TYPO3 [%s] request="%s" component="%s": %s %s', $levelName, $record->getRequestId(), $record->getComponent(), $record->getMessage(), $data);
+       public function writeLog(LogRecord $record) {
+               $levelName = LogLevel::getName($record->getLevel());
+               $data = '';
+               $recordData = $record->getData();
+               if (!empty($recordData)) {
+                       // According to PSR3 the exception-key may hold an \Exception
+                       // Since json_encode() does not encode an exception, we run the _toString() here
+                       if (isset($recordData['exception']) && $recordData['exception'] instanceof \Exception) {
+                               $recordData['exception'] = (string)$recordData['exception'];
+                       }
+                       $data = '- ' . json_encode($recordData);
+               }
+               $message = sprintf(
+                       'TYPO3 [%s] request="%s" component="%s": %s %s',
+                       $levelName,
+                       $record->getRequestId(),
+                       $record->getComponent(),
+                       $record->getMessage(),
+                       $data
+               );
                if (FALSE === error_log($message)) {
                        throw new \RuntimeException('Could not write log record to PHP error log', 1345036336);
                }
index 491c5c9..7017919 100644 (file)
@@ -13,19 +13,22 @@ namespace TYPO3\CMS\Core\Log\Writer;
  *
  * The TYPO3 project - inspiring people to share!
  */
+
+use TYPO3\CMS\Core\Log\LogRecord;
+
 /**
  * Log writer that writes to syslog
  *
  * @author Ingo Renner <ingo@typo3.org>
  * @author Steffen Müller <typo3@t3node.com>
  */
-class SyslogWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
+class SyslogWriter extends AbstractWriter {
 
        /**
         * List of valid syslog facility names.
         * private as it's not supposed to be changed.
         *
-        * @var array Facilities
+        * @var int[] Facilities
         */
        private $facilities = array(
                'auth' => LOG_AUTH,
@@ -96,24 +99,38 @@ class SyslogWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
        /**
         * Returns the data of the record in syslog format
         *
-        * @param \TYPO3\CMS\Core\Log\LogRecord $record
+        * @param LogRecord $record
         * @return string
         */
-       public function getMessageForSyslog(\TYPO3\CMS\Core\Log\LogRecord $record) {
-               $data = $record->getData();
-               $data = !empty($data) ? '- ' . json_encode($data) : '';
-               $message = sprintf('[request="%s" component="%s"] %s %s', $record->getRequestId(), $record->getComponent(), $record->getMessage(), $data);
+       public function getMessageForSyslog(LogRecord $record) {
+               $data = '';
+               $recordData = $record->getData();
+               if (!empty($recordData)) {
+                       // According to PSR3 the exception-key may hold an \Exception
+                       // Since json_encode() does not encode an exception, we run the _toString() here
+                       if (isset($recordData['exception']) && $recordData['exception'] instanceof \Exception) {
+                               $recordData['exception'] = (string)$recordData['exception'];
+                       }
+                       $data = '- ' . json_encode($recordData);
+               }
+               $message = sprintf(
+                       '[request="%s" component="%s"] %s %s',
+                       $record->getRequestId(),
+                       $record->getComponent(),
+                       $record->getMessage(),
+                       $data
+               );
                return $message;
        }
 
        /**
         * Writes the log record to syslog
         *
-        * @param \TYPO3\CMS\Core\Log\LogRecord $record Log record
+        * @param LogRecord $record Log record
         * @return \TYPO3\CMS\Core\Log\Writer\WriterInterface
         * @throws \RuntimeException
         */
-       public function writeLog(\TYPO3\CMS\Core\Log\LogRecord $record) {
+       public function writeLog(LogRecord $record) {
                if (FALSE === syslog($record->getLevel(), $this->getMessageForSyslog($record))) {
                        throw new \RuntimeException('Could not write log record to syslog', 1345036337);
                }
index 91fba65..9a8ebd9 100644 (file)
@@ -14,19 +14,22 @@ namespace TYPO3\CMS\Core\Tests\Unit\Log\Writer;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Log\Writer\DatabaseWriter;
+use TYPO3\CMS\Core\Tests\UnitTestCase;
+
 /**
  * Test case
  *
  * @author Steffen Gebert <steffen.gebert@typo3.org>
  */
-class DatabaseWriterTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
+class DatabaseWriterTest extends UnitTestCase {
 
        /**
         * @test
         */
        public function getTableReturnsPreviouslySetTable() {
                $logTable = $this->getUniqueId('logtable_');
-               /** @var \TYPO3\CMS\Core\Log\Writer\DatabaseWriter|\PHPUnit_Framework_MockObject_MockObject $subject */
+               /** @var DatabaseWriter|\PHPUnit_Framework_MockObject_MockObject $subject */
                $subject = $this->getMock('TYPO3\\CMS\Core\Log\\Writer\\DatabaseWriter', array('dummy'), array(), '', FALSE);
                $subject->setLogTable($logTable);
                $this->assertSame($logTable, $subject->getLogTable());
@@ -39,9 +42,9 @@ class DatabaseWriterTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
        public function writeLogThrowsExceptionIfDatabaseInsertFailed() {
                $GLOBALS['TYPO3_DB'] = $this->getMock('TYPO3\\CMS\\Core\\Database\\DatabaseConnection', array(), array(), '', FALSE);
                $GLOBALS['TYPO3_DB']->expects($this->once())->method('exec_INSERTquery')->will($this->returnValue(FALSE));
-               /** @var \TYPO3\CMS\Core\Log\LogRecord|\PHPUnit_Framework_MockObject_MockObject $subject */
+               /** @var \TYPO3\CMS\Core\Log\LogRecord|\PHPUnit_Framework_MockObject_MockObject $logRecordMock */
                $logRecordMock = $this->getMock('TYPO3\\CMS\\Core\\Log\\LogRecord', array(), array(), '', FALSE);
-               /** @var \TYPO3\CMS\Core\Log\Writer\DatabaseWriter|\PHPUnit_Framework_MockObject_MockObject $subject */
+               /** @var DatabaseWriter|\PHPUnit_Framework_MockObject_MockObject $subject */
                $subject = $this->getMock('TYPO3\\CMS\Core\Log\\Writer\\DatabaseWriter', array('dummy'), array(), '', FALSE);
                $subject->writeLog($logRecordMock);
        }
@@ -53,9 +56,9 @@ class DatabaseWriterTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $logTable = $this->getUniqueId('logtable_');
                $GLOBALS['TYPO3_DB'] = $this->getMock('TYPO3\\CMS\\Core\\Database\\DatabaseConnection', array(), array(), '', FALSE);
                $GLOBALS['TYPO3_DB']->expects($this->once())->method('exec_INSERTquery')->with($logTable, $this->anything());
-               /** @var \TYPO3\CMS\Core\Log\LogRecord|\PHPUnit_Framework_MockObject_MockObject $subject */
+               /** @var \TYPO3\CMS\Core\Log\LogRecord|\PHPUnit_Framework_MockObject_MockObject $logRecordMock */
                $logRecordMock = $this->getMock('TYPO3\\CMS\\Core\\Log\\LogRecord', array(), array(), '', FALSE);
-               /** @var \TYPO3\CMS\Core\Log\Writer\DatabaseWriter|\PHPUnit_Framework_MockObject_MockObject $subject */
+               /** @var DatabaseWriter|\PHPUnit_Framework_MockObject_MockObject $subject */
                $subject = $this->getMock('TYPO3\\CMS\Core\Log\\Writer\\DatabaseWriter', array('dummy'), array(), '', FALSE);
                $subject->setLogTable($logTable);
                $subject->writeLog($logRecordMock);
@@ -73,20 +76,21 @@ class DatabaseWriterTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                        'message' => $this->getUniqueId('message'),
                        'data' => '',
                );
-               /** @var \TYPO3\CMS\Core\Log\LogRecord|\PHPUnit_Framework_MockObject_MockObject $subject */
-               $logRecordMock = $this->getMock('TYPO3\\CMS\\Core\\Log\\LogRecord', array(), array(), '', FALSE);
-               $logRecordMock->expects($this->at(0))->method('offsetGet')->with('requestId')->will($this->returnValue($logRecordData['request_id']));
-               $logRecordMock->expects($this->at(1))->method('offsetGet')->with('created')->will($this->returnValue($logRecordData['time_micro']));
-               $logRecordMock->expects($this->at(2))->method('offsetGet')->with('component')->will($this->returnValue($logRecordData['component']));
-               $logRecordMock->expects($this->at(3))->method('offsetGet')->with('level')->will($this->returnValue($logRecordData['level']));
-               $logRecordMock->expects($this->at(4))->method('offsetGet')->with('message')->will($this->returnValue($logRecordData['message']));
+               /** @var \TYPO3\CMS\Core\Log\LogRecord|\PHPUnit_Framework_MockObject_MockObject $logRecordFixture */
+               $logRecordFixture = $this->getMock('TYPO3\\CMS\\Core\\Log\\LogRecord', array(), array(), '', FALSE);
+               $logRecordFixture->expects($this->any())->method('getRequestId')->will($this->returnValue($logRecordData['request_id']));
+               $logRecordFixture->expects($this->any())->method('getCreated')->will($this->returnValue($logRecordData['time_micro']));
+               $logRecordFixture->expects($this->any())->method('getComponent')->will($this->returnValue($logRecordData['component']));
+               $logRecordFixture->expects($this->any())->method('getLevel')->will($this->returnValue($logRecordData['level']));
+               $logRecordFixture->expects($this->any())->method('getMessage')->will($this->returnValue($logRecordData['message']));
+               $logRecordFixture->expects($this->any())->method('getData')->will($this->returnValue(array()));
 
-               /** @var \TYPO3\CMS\Core\Log\Writer\DatabaseWriter|\PHPUnit_Framework_MockObject_MockObject $subject */
-               $subject = $this->getMock('TYPO3\\CMS\Core\Log\\Writer\\DatabaseWriter', array('dummy'), array(), '', FALSE);
+               /** @var DatabaseWriter|\PHPUnit_Framework_MockObject_MockObject $subject */
+               $subject = new DatabaseWriter();
 
                $GLOBALS['TYPO3_DB'] = $this->getMock('TYPO3\\CMS\\Core\\Database\\DatabaseConnection', array(), array(), '', FALSE);
                $GLOBALS['TYPO3_DB']->expects($this->once())->method('exec_INSERTquery')->with($this->anything(), $logRecordData);
 
-               $subject->writeLog($logRecordMock);
+               $subject->writeLog($logRecordFixture);
        }
-}
\ No newline at end of file
+}