Skip to content
Snippets Groups Projects
Commit 34dc73da authored by Steffen Müller's avatar Steffen Müller Committed by Markus Klein
Browse files

[BUGFIX] FileLogWriter ignores log file configuration 

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/21258
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel
Reviewed-by: Markus Klein
Tested-by: Markus Klein
parent 3ad4be6d
No related merge requests found
Hide whitespace changes
Inline Side-by-side
typo3/sysext/core/Classes/Log/Writer/FileWriter.php
+ 21
− 14
View file @ 34dc73da
......@@ -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
?>
typo3/sysext/core/Tests/Unit/Log/Writer/FileTest.php
+ 48
− 11
View file @ 34dc73da
......@@ -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
?>
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment