diff --git a/src/Monolog/Formatter/NormalizerFormatter.php b/src/Monolog/Formatter/NormalizerFormatter.php index 6717e40e..8a1fdab8 100644 --- a/src/Monolog/Formatter/NormalizerFormatter.php +++ b/src/Monolog/Formatter/NormalizerFormatter.php @@ -108,8 +108,8 @@ class NormalizerFormatter implements FormatterInterface if (isset($frame['file'])) { $data['trace'][] = $frame['file'].':'.$frame['line']; } else { - $frame = $this->convertResourceArgs($frame); - $data['trace'][] = json_encode($frame); + // We should again normalize the frames, because it might contain invalid items + $data['trace'][] = $this->toJson($this->normalize($frame), true); } } @@ -137,29 +137,4 @@ class NormalizerFormatter implements FormatterInterface return json_encode($data); } - - /** - * This method checks recursively for resource args inside the frame, since json_encode is choking on them. - * - * @param array $frame Reference to current frame - * @return array - */ - private function convertResourceArgs(array $frame) - { - foreach ($frame as $key => $item) { - if (is_scalar($item)) { - continue; - } - - if (is_resource($item)) { - $frame[$key] = (string) $item; - } - - if (is_array($item)) { - $frame[$key] = $this->convertResourceArgs($item); - } - } - - return $frame; - } } diff --git a/tests/Monolog/Formatter/GelfMessageFormatterTest.php b/tests/Monolog/Formatter/GelfMessageFormatterTest.php index 2cbba3fc..3f47a09a 100644 --- a/tests/Monolog/Formatter/GelfMessageFormatterTest.php +++ b/tests/Monolog/Formatter/GelfMessageFormatterTest.php @@ -182,50 +182,6 @@ class GelfMessageFormatterTest extends \PHPUnit_Framework_TestCase $this->assertEquals('pair', $message_array['_EXTkey']); } - /** - * @covers Monolog\Formatter\GelfMessageFormatter::format - */ - public function testExceptionObjectWithResourceTrace() - { - if (defined('HHVM_VERSION')) { - $this->markTestSkipped('Not supported in HHVM since it detects errors differently'); - } - - // This happens i.e. in React promises or Guzzle streams where stream wrappers are registered - // and no file or line are included in the trace because it's treated as internal function - set_error_handler(function ($errno, $errstr, $errfile, $errline ) { - throw new \ErrorException($errstr, 0, $errno, $errfile, $errline); - }); - - try { - $resource = fopen('php://memory', '+w'); - // Just do something stupid with a resource as argument - strpos($resource, 'FOO'); - } catch (\Exception $e) { - } - - // restore the error handler - restore_error_handler(); - - $formatter = new GelfMessageFormatter(); - $record = array( - 'level' => Logger::ERROR, - 'level_name' => 'ERROR', - 'channel' => 'meh', - 'context' => array('from' => 'logger', 'exception' => $e), - 'datetime' => new \DateTime("@0"), - 'extra' => array(), - 'message' => 'log' - ); - - $message = $formatter->format($record); - - $this->assertRegExp( - '%\\\\"resource\\\\":\\\\"Resource id #\d+\\\\"%', - $message->getAdditional('ctxt_exception') - ); - } - private function isLegacy() { return interface_exists('\Gelf\IMessagePublisher'); diff --git a/tests/Monolog/Formatter/NormalizerFormatterTest.php b/tests/Monolog/Formatter/NormalizerFormatterTest.php index 74778712..859fe25d 100644 --- a/tests/Monolog/Formatter/NormalizerFormatterTest.php +++ b/tests/Monolog/Formatter/NormalizerFormatterTest.php @@ -166,6 +166,51 @@ class NormalizerFormatterTest extends \PHPUnit_Framework_TestCase $this->assertEquals(@json_encode(array($resource)), $res); } + + public function testExceptionTraceWithArgs() + { + if (defined('HHVM_VERSION')) { + $this->markTestSkipped('Not supported in HHVM since it detects errors differently'); + } + + // This happens i.e. in React promises or Guzzle streams where stream wrappers are registered + // and no file or line are included in the trace because it's treated as internal function + set_error_handler(function ($errno, $errstr, $errfile, $errline) { + throw new \ErrorException($errstr, 0, $errno, $errfile, $errline); + }); + + try { + // This will contain $resource and $wrappedResource as arguments in the trace item + $resource = fopen('php://memory', 'rw+'); + fwrite($resource, 'test_resource'); + $wrappedResource = new TestStreamFoo($resource); + // Just do something stupid with a resource/wrapped resource as argument + array_keys($wrappedResource); + } catch (\Exception $e) { + restore_error_handler(); + } + + $formatter = new NormalizerFormatter(); + $record = array('context' => array('exception' => $e)); + $result = $formatter->format($record); + + $this->assertRegExp( + '%"resource":"\[resource\]"%', + $result['context']['exception']['trace'][0] + ); + + if (version_compare(PHP_VERSION, '5.5.0', '>=')) { + $pattern = '%"wrappedResource":"\[object\] \(Monolog\\\\\\\\Formatter\\\\\\\\TestStreamFoo: \)"%'; + } else { + $pattern = '%\\\\"resource\\\\":null%'; + } + + // Tests that the wrapped resource is ignored while encoding, only works for PHP <= 5.4 + $this->assertRegExp( + $pattern, + $result['context']['exception']['trace'][0] + ); + } } class TestFooNorm @@ -180,3 +225,21 @@ class TestBarNorm return 'bar'; } } + +class TestStreamFoo +{ + public $foo; + public $resource; + + public function __construct($resource) + { + $this->resource = $resource; + $this->foo = 'BAR'; + } + + public function __toString() + { + fseek($this->resource, 0); + return $this->foo . ' - ' . (string) stream_get_contents($this->resource); + } +}