Navigation Menu

Skip to content

Commit

Permalink
bug #31326 fix ConsoleFormatter - call to a member function format() …
Browse files Browse the repository at this point in the history
…on string (keksa)

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
  • Loading branch information
fabpot committed May 1, 2019
2 parents 885d08c + 6488328 commit e0b5fb2
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/Symfony/Bridge/Monolog/Formatter/ConsoleFormatter.php
Expand Up @@ -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%' => '</>',
Expand Down
@@ -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",
],
];
}
}

0 comments on commit e0b5fb2

Please sign in to comment.