From 1841e2ba88bd89df56277a1402aebb7ab61eef7b Mon Sep 17 00:00:00 2001 From: Remon van de Kamp Date: Mon, 18 Apr 2016 21:51:37 +0200 Subject: [PATCH] Lock down RotateFileHandler to prevent errors with rotation - Require the dateFormat to be one of three presets ('Y-m-d', 'Y-m' or 'Y') to ensure that dates can be sorted lexographically - Require the filenameFormat to contain the (sub)string `{date}` so we will actually create new files instead of the same file over and over again. --- src/Monolog/Handler/RotatingFileHandler.php | 17 ++++ .../Handler/RotatingFileHandlerTest.php | 92 +++++++++++++++++-- 2 files changed, 99 insertions(+), 10 deletions(-) diff --git a/src/Monolog/Handler/RotatingFileHandler.php b/src/Monolog/Handler/RotatingFileHandler.php index 22e6c3dc..893a5cb3 100644 --- a/src/Monolog/Handler/RotatingFileHandler.php +++ b/src/Monolog/Handler/RotatingFileHandler.php @@ -11,6 +11,7 @@ namespace Monolog\Handler; +use InvalidArgumentException; use Monolog\Logger; /** @@ -24,6 +25,10 @@ use Monolog\Logger; */ class RotatingFileHandler extends StreamHandler { + const FILE_PER_DAY = 'Y-m-d'; + const FILE_PER_MONTH = 'Y-m'; + const FILE_PER_YEAR = 'Y'; + protected $filename; protected $maxFiles; protected $mustRotate; @@ -64,6 +69,18 @@ class RotatingFileHandler extends StreamHandler public function setFilenameFormat($filenameFormat, $dateFormat) { + if (!in_array($dateFormat, array(self::FILE_PER_DAY, self::FILE_PER_MONTH, self::FILE_PER_YEAR))) { + throw new InvalidArgumentException( + 'Invalid date format - format must be one of '. + 'RotatingFileHandler::FILE_PER_DAY, RotatingFileHandler::FILE_PER_MONTH '. + 'or RotatingFileHandler::FILE_PER_YEAR.' + ); + } + if (substr_count($filenameFormat, '{date}') === 0) { + throw new InvalidArgumentException( + 'Invalid filename format - format must contain at least `{date}`, because otherwise rotating is impossible.' + ); + } $this->filenameFormat = $filenameFormat; $this->dateFormat = $dateFormat; $this->url = $this->getTimedFilename(); diff --git a/tests/Monolog/Handler/RotatingFileHandlerTest.php b/tests/Monolog/Handler/RotatingFileHandlerTest.php index f4cefda1..44413657 100644 --- a/tests/Monolog/Handler/RotatingFileHandlerTest.php +++ b/tests/Monolog/Handler/RotatingFileHandlerTest.php @@ -11,6 +11,7 @@ namespace Monolog\Handler; +use InvalidArgumentException; use Monolog\TestCase; /** @@ -23,7 +24,7 @@ class RotatingFileHandlerTest extends TestCase $dir = __DIR__.'/Fixtures'; chmod($dir, 0777); if (!is_writable($dir)) { - $this->markTestSkipped($dir.' must be writeable to test the RotatingFileHandler.'); + $this->markTestSkipped($dir.' must be writable to test the RotatingFileHandler.'); } } @@ -43,14 +44,14 @@ class RotatingFileHandlerTest extends TestCase /** * @dataProvider rotationTests */ - public function testRotation($createFile) + public function testRotation($createFile, $dateFormat, $timeCallback) { - touch($old1 = __DIR__.'/Fixtures/foo-'.date('Y-m-d', time() - 86400).'.rot'); - touch($old2 = __DIR__.'/Fixtures/foo-'.date('Y-m-d', time() - 86400 * 2).'.rot'); - touch($old3 = __DIR__.'/Fixtures/foo-'.date('Y-m-d', time() - 86400 * 3).'.rot'); - touch($old4 = __DIR__.'/Fixtures/foo-'.date('Y-m-d', time() - 86400 * 4).'.rot'); + touch($old1 = __DIR__.'/Fixtures/foo-'.date($dateFormat, $timeCallback(-1)).'.rot'); + touch($old2 = __DIR__.'/Fixtures/foo-'.date($dateFormat, $timeCallback(-2)).'.rot'); + touch($old3 = __DIR__.'/Fixtures/foo-'.date($dateFormat, $timeCallback(-3)).'.rot'); + touch($old4 = __DIR__.'/Fixtures/foo-'.date($dateFormat, $timeCallback(-4)).'.rot'); - $log = __DIR__.'/Fixtures/foo-'.date('Y-m-d').'.rot'; + $log = __DIR__.'/Fixtures/foo-'.date($dateFormat).'.rot'; if ($createFile) { touch($log); @@ -58,6 +59,7 @@ class RotatingFileHandlerTest extends TestCase $handler = new RotatingFileHandler(__DIR__.'/Fixtures/foo.rot', 2); $handler->setFormatter($this->getIdentityFormatter()); + $handler->setFilenameFormat('{filename}-{date}', $dateFormat); $handler->handle($this->getRecord()); $handler->close(); @@ -72,11 +74,81 @@ class RotatingFileHandlerTest extends TestCase public function rotationTests() { + $now = time(); + $dayCallback = function($ago) use ($now) { + return $now + 86400 * $ago; + }; + $monthCallback = function($ago) { + return gmmktime(0, 0, 0, date('n') + $ago, date('d'), date('Y')); + }; + $yearCallback = function($ago) { + return gmmktime(0, 0, 0, date('n'), date('d'), date('Y') + $ago); + }; + return array( 'Rotation is triggered when the file of the current day is not present' - => array(true), - 'Rotation is not triggered when the file is already present' - => array(false), + => array(true, RotatingFileHandler::FILE_PER_DAY, $dayCallback), + 'Rotation is not triggered when the file of the current day is already present' + => array(false, RotatingFileHandler::FILE_PER_DAY, $dayCallback), + + 'Rotation is triggered when the file of the current month is not present' + => array(true, RotatingFileHandler::FILE_PER_MONTH, $monthCallback), + 'Rotation is not triggered when the file of the current month is already present' + => array(false, RotatingFileHandler::FILE_PER_MONTH, $monthCallback), + + 'Rotation is triggered when the file of the current year is not present' + => array(true, RotatingFileHandler::FILE_PER_YEAR, $yearCallback), + 'Rotation is not triggered when the file of the current year is already present' + => array(false, RotatingFileHandler::FILE_PER_YEAR, $yearCallback), + ); + } + + /** + * @dataProvider dateFormatProvider + */ + public function testAllowOnlyFixedDefinedDateFormats($dateFormat, $valid) + { + $handler = new RotatingFileHandler(__DIR__.'/Fixtures/foo.rot', 2); + if (!$valid) { + $this->setExpectedExceptionRegExp(InvalidArgumentException::class, '~^Invalid date format~'); + } + $handler->setFilenameFormat('{filename}-{date}', $dateFormat); + } + + public function dateFormatProvider() + { + return array( + array(RotatingFileHandler::FILE_PER_DAY, true), + array(RotatingFileHandler::FILE_PER_MONTH, true), + array(RotatingFileHandler::FILE_PER_YEAR, true), + array('m-d-Y', false), + array('Y-m-d-h-i', false) + ); + } + + /** + * @dataProvider filenameFormatProvider + */ + public function testDisallowFilenameFormatsWithoutDate($filenameFormat, $valid) + { + $handler = new RotatingFileHandler(__DIR__.'/Fixtures/foo.rot', 2); + if (!$valid) { + $this->setExpectedExceptionRegExp(InvalidArgumentException::class, '~^Invalid filename format~'); + } + + $handler->setFilenameFormat($filenameFormat, RotatingFileHandler::FILE_PER_DAY); + } + + public function filenameFormatProvider() + { + return array( + array('{filename}', false), + array('{filename}-{date}', true), + array('{date}', true), + array('foobar-{date}', true), + array('foo-{date}-bar', true), + array('{date}-foobar', true), + array('foobar', false), ); }