Resolve json encoding errors issue globally, refs #137

Seldaek · Jan 6, 2013 · 63e3bfd · 63e3bfd
1 parent 271c396
commit 63e3bfd
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 52 deletions.
diff --git a/src/Monolog/Formatter/LineFormatter.php b/src/Monolog/Formatter/LineFormatter.php
@@ -81,10 +81,11 @@ protected function convertToString($data)
             return (string) $data;
         }
 
+        $data = $this->normalize($data);
         if (version_compare(PHP_VERSION, '5.4.0', '>=')) {
-            return json_encode($this->normalize($data), JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
+            return $this->toJson($data);
         }
 
-        return str_replace('\\/', '/', json_encode($this->normalize($data)));
+        return str_replace('\\/', '/', json_encode($data));
     }
 }
diff --git a/src/Monolog/Formatter/NormalizerFormatter.php b/src/Monolog/Formatter/NormalizerFormatter.php
@@ -71,7 +71,7 @@ protected function normalize($data)
         }
 
         if (is_object($data)) {
-            return sprintf("[object] (%s: %s)", get_class($data), $this->toJson($data));
+            return sprintf("[object] (%s: %s)", get_class($data), $this->toJson($data, true));
         }
 
         if (is_resource($data)) {
@@ -81,8 +81,17 @@ protected function normalize($data)
         return '[unknown('.gettype($data).')]';
     }
 
-    protected function toJson($data)
+    protected function toJson($data, $ignoreErrors = false)
     {
+        // suppress json_encode errors since it's twitchy with some inputs
+        if ($ignoreErrors) {
+            if (version_compare(PHP_VERSION, '5.4.0', '>=')) {
+                return @json_encode($data, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
+            }
+
+            return @json_encode($data);
+        }
+
         if (version_compare(PHP_VERSION, '5.4.0', '>=')) {
             return json_encode($data, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
         }

diff --git a/src/Monolog/Formatter/WildfireFormatter.php b/src/Monolog/Formatter/WildfireFormatter.php
@@ -67,25 +67,16 @@ public function format(array $record)
             $message = reset($message);
         }
 
-        // Handle json_encode recursion error
-        if ($handleError) {
-            set_error_handler(function () { });
-        }
-
         // Create JSON object describing the appearance of the message in the console
-        $json = json_encode(array(
+        $json = $this->toJson(array(
             array(
                 'Type'  => $this->logLevels[$record['level']],
                 'File'  => $file,
                 'Line'  => $line,
                 'Label' => $record['channel'],
             ),
             $message,
-        ));
-
-        if ($handleError) {
-            restore_error_handler();
-        }
+        ), $handleError);
 
         // The message itself is a serialization of the above JSON object + it's length
         return sprintf(

diff --git a/tests/Monolog/Formatter/NormalizerFormatterTest.php b/tests/Monolog/Formatter/NormalizerFormatterTest.php
@@ -89,6 +89,61 @@ public function testBatchFormat()
             ),
         ), $formatted);
     }
+
+    /**
+     * Test issue #137
+     */
+    public function testIgnoresRecursiveObjectReferences()
+    {
+        // set up the recursion
+        $foo = new \stdClass();
+        $bar = new \stdClass();
+
+        $foo->bar = $bar;
+        $bar->foo = $foo;
+
+        // set an error handler to assert that the error is not raised anymore
+        $that = $this;
+        set_error_handler(function ($level, $message, $file, $line, $context) use ($that) {
+            if (error_reporting() & $level) {
+                restore_error_handler();
+                $that->fail("$message should not be raised");
+            }
+        });
+
+        $formatter = new NormalizerFormatter();
+        $reflMethod = new \ReflectionMethod($formatter, 'toJson');
+        $reflMethod->setAccessible(true);
+        $res = $reflMethod->invoke($formatter, array($foo, $bar), true);
+
+        restore_error_handler();
+
+        $this->assertEquals(@json_encode(array($foo, $bar)), $res);
+    }
+
+    public function testIgnoresInvalidTypes()
+    {
+        // set up the recursion
+        $resource = fopen(__FILE__, 'r');
+
+        // set an error handler to assert that the error is not raised anymore
+        $that = $this;
+        set_error_handler(function ($level, $message, $file, $line, $context) use ($that) {
+            if (error_reporting() & $level) {
+                restore_error_handler();
+                $that->fail("$message should not be raised");
+            }
+        });
+
+        $formatter = new NormalizerFormatter();
+        $reflMethod = new \ReflectionMethod($formatter, 'toJson');
+        $reflMethod->setAccessible(true);
+        $res = $reflMethod->invoke($formatter, array($resource), true);
+
+        restore_error_handler();
+
+        $this->assertEquals(@json_encode(array($resource)), $res);
+    }
 }
 
 class TestFooNorm

diff --git a/tests/Monolog/Formatter/WildfireFormatterTest.php b/tests/Monolog/Formatter/WildfireFormatterTest.php
@@ -108,41 +108,4 @@ public function testBatchFormatThrowException()
 
         $wildfire->formatBatch(array($record));
     }
-
-    /**
-     * Test issue #137 (https://github.com/Seldaek/monolog/pull/137)
-     */
-    public function testFormatWithObjectsInContext()
-    {
-        // Set up the recursion
-        $foo = new \stdClass();
-        $bar = new \stdClass();
-
-        $foo->bar = $bar;
-        $bar->foo = $foo;
-
-        $record = array(
-            'message'    => "foo",
-            'level'      => 300,
-            'channel'    => 'foo',
-            'context'    => array(
-                'stack'  => array(
-                    array($foo),
-                    array($bar),
-                ),
-            ),
-            'extra'      => array(),
-        );
-
-        // Set an error handler to assert that the error is not raised anymore
-        $that = $this;
-        set_error_handler(function ($level, $message, $file, $line, $context) use ($that) {
-           $that->fail("$message should not be raised anymore");
-        });
-
-        $wildfire = new WildfireFormatter();
-        $wildfire->format($record);
-
-        restore_error_handler();
-    }
 }