[BUGFIX] FileLogWriter ignores log file configuration 25/21325/2
authorSteffen Müller <typo3@t3node.com>
Thu, 6 Jun 2013 13:42:17 +0000 (15:42 +0200)
committerMarkus Klein <klein.t3@mfc-linz.at>
Thu, 13 Jun 2013 15:37:10 +0000 (17:37 +0200)
If there are several instances of TYPO3\CMS\Core\Log\Writer\FileLogWriter
with different log files configured in $logFile, all log records end up
in one file.

This is caused by improper use of static variable $logFileHandle.
All filehandles except the one of the latest instance are ignored.

Resolves: #48918
Releases: 6.2, 6.1, 6.0
Change-Id: Ie6de5e4789d107b541117daf6c7e9855015e0a46
Reviewed-on: https://review.typo3.org/21325
Reviewed-by: Markus Klein
Tested-by: Markus Klein
typo3/sysext/core/Classes/Log/Writer/FileWriter.php
typo3/sysext/core/Tests/Unit/Log/Writer/FileTest.php

index 792c1d4..372b11a 100644 (file)
@@ -49,12 +49,15 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
        protected $defaultLogFile = 'typo3temp/logs/typo3.log';
 
        /**
-        * Log file handle
+        * Log file handle storage
+        *
+        * To avoid concurrent file handles on a the same file when using several FileWriter instances,
+        * we share the file handles in a static class variable
         *
         * @static
-        * @var resource
+        * @var array
         */
-       static protected $logFileHandle = NULL;
+       static protected $logFileHandles = array();
 
        /**
         * Constructor, opens the log file handle
@@ -85,11 +88,8 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
         * @throws \InvalidArgumentException
         */
        public function setLogFile($logFile) {
-               if (is_resource(self::$logFileHandle)) {
-                       $this->closeLogFile();
-               }
-               // Skip handling if logFile is a stream resource
-               // This is used by unit tests with vfs:// directories
+
+               // 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))) {
                                throw new \InvalidArgumentException('Log file path "' . $logFile . '" is not valid!', 1326411176);
@@ -98,6 +98,7 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
                }
                $this->logFile = $logFile;
                $this->openLogFile();
+
                return $this;
        }
 
@@ -118,9 +119,10 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
         * @throws \RuntimeException
         */
        public function writeLog(\TYPO3\CMS\Core\Log\LogRecord $record) {
-               if (FALSE === fwrite(self::$logFileHandle, $record . LF)) {
+               if (FALSE === fwrite(self::$logFileHandles[$this->logFile], $record . LF)) {
                        throw new \RuntimeException('Could not write log record to log file', 1345036335);
                }
+
                return $this;
        }
 
@@ -131,9 +133,13 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
         * @throws \RuntimeException if the log file can't be opened.
         */
        protected function openLogFile() {
+               if (is_resource(self::$logFileHandles[$this->logFile])) {
+                       return;
+               }
+
                $this->createLogFile();
-               self::$logFileHandle = fopen($this->logFile, 'a');
-               if (!is_resource(self::$logFileHandle)) {
+               self::$logFileHandles[$this->logFile] = fopen($this->logFile, 'a');
+               if (!is_resource(self::$logFileHandles[$this->logFile])) {
                        throw new \RuntimeException('Could not open log file "' . $this->logFile . '"', 1321804422);
                }
        }
@@ -144,8 +150,9 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
         * @return void
         */
        protected function closeLogFile() {
-               if (is_resource(self::$logFileHandle)) {
-                       fclose(self::$logFileHandle);
+               if (is_resource(self::$logFileHandles[$this->logFile])) {
+                       fclose(self::$logFileHandles[$this->logFile]);
+                       unset(self::$logFileHandles[$this->logFile]);
                }
        }
 
@@ -185,4 +192,4 @@ class FileWriter extends \TYPO3\CMS\Core\Log\Writer\AbstractWriter {
 }
 
 
-?>
\ No newline at end of file
+?>
index 01d4571..5e53285 100644 (file)
@@ -68,18 +68,19 @@ class FileTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
        /**
         * Creates a file writer
         *
+        * @param string $prependName
         * @return \TYPO3\CMS\Core\Log\Writer\FileWriter
         */
-       protected function createWriter() {
+       protected function createWriter($prependName = '') {
                /** @var \TYPO3\CMS\Core\Log\Writer\FileWriter $writer */
                $writer = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Core\\Log\\Writer\\FileWriter', array(
-                       'logFile' => 'vfs://LogRoot/' . $this->logFileDirectory . '/' . $this->logFileName
+                       'logFile' => $this->getDefaultFileName($prependName)
                ));
                return $writer;
        }
 
-       protected function getDefaultFileName() {
-               return 'vfs://LogRoot/' . $this->logFileDirectory . '/' . $this->logFileName;
+       protected function getDefaultFileName($prependName = '') {
+               return 'vfs://LogRoot/' . $this->logFileDirectory . '/' . $prependName . $this->logFileName;
        }
 
        /**
@@ -127,8 +128,8 @@ class FileTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $simpleRecord = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Core\\Log\\LogRecord', uniqid('test.core.log.fileWriter.simpleRecord.'), \TYPO3\CMS\Core\Log\LogLevel::INFO, 'test record');
                $recordWithData = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Core\\Log\\LogRecord', uniqid('test.core.log.fileWriter.recordWithData.'), \TYPO3\CMS\Core\Log\LogLevel::ALERT, 'test record with data', array('foo' => array('bar' => 'baz')));
                return array(
-                       'simple record' => array($simpleRecord, (string) $simpleRecord),
-                       'record with data' => array($recordWithData, (string) $recordWithData)
+                       'simple record' => array($simpleRecord, trim((string) $simpleRecord)),
+                       'record with data' => array($recordWithData, trim((string) $recordWithData))
                );
        }
 
@@ -141,10 +142,46 @@ class FileTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
        public function logsToFile(\TYPO3\CMS\Core\Log\LogRecord $record, $expectedResult) {
                $this->setUpVfsStream();
                $this->createWriter()->writeLog($record);
-               $logFileContents = file_get_contents($this->getDefaultFileName());
-               $logFileContents = trim($logFileContents);
-               $expectedResult = trim($expectedResult);
-               $this->assertEquals($logFileContents, $expectedResult);
+               $logFileContents = trim(file_get_contents($this->getDefaultFileName()));
+               $this->assertEquals($expectedResult, $logFileContents);
+       }
+
+       /**
+        * @test
+        * @param \TYPO3\CMS\Core\Log\LogRecord $record Record Test Data
+        * @param string $expectedResult Needle
+        * @dataProvider logsToFileDataProvider
+        */
+       public function differentWritersLogToDifferentFiles(\TYPO3\CMS\Core\Log\LogRecord $record, $expectedResult) {
+               $this->setUpVfsStream();
+               $firstWriter = $this->createWriter();
+               $secondWriter = $this->createWriter('second-');
+
+               $firstWriter->writeLog($record);
+               $secondWriter->writeLog($record);
+
+               $firstLogFileContents = trim(file_get_contents($this->getDefaultFileName()));
+               $secondLogFileContents = trim(file_get_contents($this->getDefaultFileName('second-')));
+
+               $this->assertEquals($expectedResult, $firstLogFileContents);
+               $this->assertEquals($expectedResult, $secondLogFileContents);
+       }
+
+       /**
+        * @test
+        */
+       public function aSecondLogWriterToTheSameFileDoesNotOpenTheFileTwice() {
+               $this->setUpVfsStream();
+
+               $firstWriter = $this->getMock('TYPO3\\CMS\\Core\\Log\\Writer\\FileWriter', array('dummy'));
+               $secondWriter = $this->getMock('TYPO3\\CMS\\Core\\Log\\Writer\\FileWriter', array('createLogFile'));
+
+               $secondWriter->expects($this->never())->method('createLogFile');
+
+               $logFilePrefix = uniqid('unique');
+               $firstWriter->setLogFile($this->getDefaultFileName($logFilePrefix));
+               $secondWriter->setLogFile($this->getDefaultFileName($logFilePrefix));
+
        }
 
        /**
@@ -174,4 +211,4 @@ class FileTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
 
 }
 
-?>
\ No newline at end of file
+?>