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

@@ -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();

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),
         );
     }