[FOLLOWUP][BUGFIX] Handle exceptions in Logging API 38/37638/3
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:18:47 +0000 (23:18 +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/37638
Reviewed-by: Frank Nägler <typo3@naegler.net>
Tested-by: Frank Nägler <typo3@naegler.net>
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 2a851ac..1dbfef5 100644 (file)
@@ -261,7 +261,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 98714ad..a082122 100644 (file)
@@ -13,6 +13,7 @@ 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.
@@ -51,23 +52,42 @@ class DatabaseWriter 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) {
-               $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 0f191d5..10ed469 100644 (file)
@@ -15,6 +15,7 @@ namespace TYPO3\CMS\Core\Log\Writer;
  */
 
 use TYPO3\CMS\Core\Log\Exception\InvalidLogWriterConfigurationException;
+use TYPO3\CMS\Core\Log\LogLevel;
 use TYPO3\CMS\Core\Log\LogRecord;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
@@ -112,7 +113,30 @@ class FileWriter extends 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);
                }
 
index 89fd750..19bb59c 100644 (file)
@@ -13,6 +13,7 @@ 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
@@ -24,10 +25,10 @@ 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 8b4ce2f..6e29c4a 100644 (file)
@@ -13,6 +13,8 @@ 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.
@@ -25,15 +27,30 @@ 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 0b6c0fb..e4b9250 100644 (file)
@@ -13,6 +13,7 @@ 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
@@ -26,7 +27,7 @@ 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,
@@ -97,24 +98,38 @@ class SyslogWriter extends 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 fca0718..4133d0d 100644 (file)
@@ -13,6 +13,7 @@ namespace TYPO3\CMS\Core\Tests\Unit\Log\Writer;
  *
  * The TYPO3 project - inspiring people to share!
  */
+use TYPO3\CMS\Core\Log\LogRecord;
 
 /**
  * Test case
@@ -74,20 +75,22 @@ class DatabaseWriterTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                        'data' => '',
                );
                /** @var \TYPO3\CMS\Core\Log\LogRecord|\PHPUnit_Framework_MockObject_MockObject $subject */
-               $logRecordMock = $this->getMock(\TYPO3\CMS\Core\Log\LogRecord::class, 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']));
+               $logRecordFixture = $this->getMock(\TYPO3\CMS\Core\Log\LogRecord::class, 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::class, array('dummy'), array(), '', FALSE);
+               #$subject = $this->getMock(\TYPO3\CMS\Core\Log\Writer\DatabaseWriter::class, array('dummy'), array(), '', FALSE);
+               $subject = new \TYPO3\CMS\Core\Log\Writer\DatabaseWriter();
 
                $GLOBALS['TYPO3_DB'] = $this->getMock(\TYPO3\CMS\Core\Database\DatabaseConnection::class, 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