bug symfony#31326 fix ConsoleFormatter - call to a member function format() on string (keksa)

fabpot · fabpot · commit e0b5fb2aa31b · 2019-05-01T08:30:15.000+02:00
This PR was merged into the 3.4 branch. Discussion ---------- fix ConsoleFormatter - call to a member function format() on string | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | maybe | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - The ConsoleFormatter crashes when there is not a DateTime object in `$record['datetime']`. As this parameter is not documented anywhere (i.e. `FormatterInterface` does not say it must be a DateTime object), I think the proper fix is to check if there is DateTimeInterface object and only call the `format` method then. We use a custom LogProcessor (https://symfony.com/doc/current/logging/processors.html) to add some extra data and format the DateTime in the `$record['datetime']`. We need to format the DateTime in the processor, because we use `JsonFormatter` in prod environment and it does not support changing the date format. We use `ConsoleFormatter` only in dev environment and as the processor is called before the formatter, we get the crash. There were no tests whatsoever for `ConsoleFormatter`, so I've added a basic one, that passes before and after, and another one that tests the crash (failed before, passed after). There is a theoretical BC break, as someone could have sent an object with a `format` method to the formatter and it would have worked, but I'm not sure if it's considered BC break by Symfony, please let me know, if it is. Commits ------- 6488328 fix ConsoleFormatter - call to a member function format() on string
diff --git a/src/Symfony/Bridge/Monolog/Formatter/ConsoleFormatter.php b/src/Symfony/Bridge/Monolog/Formatter/ConsoleFormatter.php
@@ -133,7 +133,9 @@ public function format(array $record)
         }
 
         $formatted = strtr($this->options['format'], [
-            '%datetime%' => $record['datetime']->format($this->options['date_format']),
+            '%datetime%' => $record['datetime'] instanceof \DateTimeInterface
+                ? $record['datetime']->format($this->options['date_format'])
+                : $record['datetime'],
             '%start_tag%' => sprintf('<%s>', $levelColor),
             '%level_name%' => sprintf('%-9s', $record['level_name']),
             '%end_tag%' => '</>',
diff --git a/src/Symfony/Bridge/Monolog/Tests/Formatter/ConsoleFormatterTest.php b/src/Symfony/Bridge/Monolog/Tests/Formatter/ConsoleFormatterTest.php
@@ -0,0 +1,66 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bridge\Monolog\Tests\Formatter;
+
+use Monolog\Logger;
+use PHPUnit\Framework\TestCase;
+use Symfony\Bridge\Monolog\Formatter\ConsoleFormatter;
+
+class ConsoleFormatterTest extends TestCase
+{
+    /**
+     * @dataProvider providerFormatTests
+     */
+    public function testFormat(array $record, $expectedMessage)
+    {
+        $formatter = new ConsoleFormatter();
+        self::assertSame($expectedMessage, $formatter->format($record));
+    }
+
+    /**
+     * @return array
+     */
+    public function providerFormatTests()
+    {
+        $currentDateTime = new \DateTime();
+
+        return [
+            'record with DateTime object in datetime field' => [
+                'record' => [
+                    'message' => 'test',
+                    'context' => [],
+                    'level' => Logger::WARNING,
+                    'level_name' => Logger::getLevelName(Logger::WARNING),
+                    'channel' => 'test',
+                    'datetime' => $currentDateTime,
+                    'extra' => [],
+                ],
+                'expectedMessage' => sprintf(
+                    "%s <fg=cyan>WARNING  </> <comment>[test]</> test\n",
+                    $currentDateTime->format(ConsoleFormatter::SIMPLE_DATE)
+                ),
+            ],
+            'record with string in datetime field' => [
+                'record' => [
+                    'message' => 'test',
+                    'context' => [],
+                    'level' => Logger::WARNING,
+                    'level_name' => Logger::getLevelName(Logger::WARNING),
+                    'channel' => 'test',
+                    'datetime' => '2019-01-01T00:42:00+00:00',
+                    'extra' => [],
+                ],
+                'expectedMessage' => "2019-01-01T00:42:00+00:00 <fg=cyan>WARNING  </> <comment>[test]</> test\n",
+            ],
+        ];
+    }
+}
