Normalize the way backtraces are normalized and remove args for speci…

…al cases, fixes Seldaek#1346
siwinski · Aug 16, 2019 · 6688b45 · 6688b45
1 parent c35fbc2
commit 6688b45
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
   * Added support for RFC3164 (outdated BSD syslog protocol) to SyslogUdpHandler
   * Fixed issue in GroupHandler and WhatFailureGroupHandler where setting multiple processors would duplicate records
   * Fixed issue in SignalHandler restarting syscalls functionality
+  * Fixed normalizers handling of exception backtraces to avoid serializing arguments in some cases
 
 ### 1.24.0 (2018-11-05)
 

diff --git a/src/Monolog/Formatter/JsonFormatter.php b/src/Monolog/Formatter/JsonFormatter.php
@@ -195,12 +195,8 @@ protected function normalizeException($e)
             foreach ($trace as $frame) {
                 if (isset($frame['file'])) {
                     $data['trace'][] = $frame['file'].':'.$frame['line'];
-                } elseif (isset($frame['function']) && $frame['function'] === '{closure}') {
-                    // We should again normalize the frames, because it might contain invalid items
-                    $data['trace'][] = $frame['function'];
                 } else {
-                    // We should again normalize the frames, because it might contain invalid items
-                    $data['trace'][] = $this->normalize($frame);
+                    $data['trace'][] = (!empty($frame['class']) ? $frame['class'].$frame['type'] : '').$frame['function'];
                 }
             }
         }

diff --git a/src/Monolog/Formatter/NormalizerFormatter.php b/src/Monolog/Formatter/NormalizerFormatter.php
@@ -151,23 +151,8 @@ protected function normalizeException($e)
         foreach ($trace as $frame) {
             if (isset($frame['file'])) {
                 $data['trace'][] = $frame['file'].':'.$frame['line'];
-            } elseif (isset($frame['function']) && $frame['function'] === '{closure}') {
-                // Simplify closures handling
-                $data['trace'][] = $frame['function'];
             } else {
-                if (isset($frame['args'])) {
-                    // Make sure that objects present as arguments are not serialized nicely but rather only
-                    // as a class name to avoid any unexpected leak of sensitive information
-                    $frame['args'] = array_map(function ($arg) {
-                        if (is_object($arg) && !($arg instanceof \DateTime || $arg instanceof \DateTimeInterface)) {
-                            return sprintf("[object] (%s)", Utils::getClass($arg));
-                        }
-
-                        return $arg;
-                    }, $frame['args']);
-                }
-                // We should again normalize the frames, because it might contain invalid items
-                $data['trace'][] = $this->toJson($this->normalize($frame), true);
+                $data['trace'][] = (!empty($frame['class']) ? $frame['class'].$frame['type'] : '').$frame['function'];
             }
         }
 

diff --git a/tests/Monolog/Formatter/NormalizerFormatterTest.php b/tests/Monolog/Formatter/NormalizerFormatterTest.php
@@ -390,21 +390,12 @@ public function testExceptionTraceWithArgs()
         $record = array('context' => array('exception' => $e));
         $result = $formatter->format($record);
 
-        $this->assertRegExp(
-            '%"resource":"\[resource\] \(stream\)"%',
-            $result['context']['exception']['trace'][0]
-        );
-
-        if (version_compare(PHP_VERSION, '5.5.0', '>=')) {
-            $pattern = '%"wrappedResource":"\[object\] \(Monolog\\\\\\\\Formatter\\\\\\\\TestFooNorm: \)"%';
-        } else {
-            $pattern = '%\\\\"foo\\\\":null%';
-        }
-
-        // Tests that the wrapped resource is ignored while encoding, only works for PHP <= 5.4
-        $this->assertRegExp(
-            $pattern,
-            $result['context']['exception']['trace'][0]
+        $this->assertSame(
+            array(
+                PHP_VERSION_ID < 50400 ? 'Monolog\Formatter\{closure}' : 'Monolog\Formatter\NormalizerFormatterTest->Monolog\Formatter\{closure}',
+                __FILE__.':'.(__LINE__-12),
+            ),
+            array_slice($result['context']['exception']['trace'], 0, 2)
         );
     }
 
@@ -421,7 +412,7 @@ public function testExceptionTraceDoesNotLeakCallUserFuncArgs()
         $result = $formatter->format($record);
 
         $this->assertSame(
-            '{"function":"throwHelper","class":"Monolog\\\\Formatter\\\\NormalizerFormatterTest","type":"->","args":["[object] (Monolog\\\\Formatter\\\\TestInfoLeak)","'.$dt->format('Y-m-d H:i:s').'"]}',
+            'Monolog\\Formatter\\NormalizerFormatterTest->throwHelper',
             $result['context']['exception']['trace'][0]
         );
     }

diff --git a/tests/Monolog/Formatter/ScalarFormatterTest.php b/tests/Monolog/Formatter/ScalarFormatterTest.php
@@ -28,7 +28,7 @@ public function buildTrace(\Exception $e)
             if (isset($frame['file'])) {
                 $data[] = $frame['file'].':'.$frame['line'];
             } else {
-                $data[] = json_encode($frame);
+                $data[] = (!empty($frame['class']) ? $frame['class'].$frame['type'] : '').$frame['function'];
             }
         }