Merge pull request #3444 from Icinga/bugfix/error-responding-non-html-requests-2635

Json::encode(): auto-sanitize bad UTF-8 strings

library/Icinga/Application/Logger.php

@@ -9,6 +9,7 @@ use Icinga\Application\Logger\Writer\FileWriter;
 use Icinga\Application\Logger\Writer\SyslogWriter;
 use Icinga\Exception\ConfigurationError;
 use Icinga\Exception\IcingaException;
+use Icinga\Util\Json;
 
 /**
  * Logger
@@ -251,7 +252,7 @@ class Logger
                 function ($a) {
                     return is_string($a) ? $a : ($a instanceof Exception
                         ? IcingaException::describe($a)
-                        : json_encode($a));
+                        : Json::encode($a));
                 },
                 $arguments
             )

library/Icinga/Util/Json.php

@@ -22,16 +22,58 @@ class Json
      * @throws  JsonEncodeException
      */
     public static function encode($value, $options = 0, $depth = 512)
+    {
+        return static::encodeAndSanitize($value, $options, $depth, false);
+    }
+
+    /**
+     * {@link json_encode()} wrapper, automatically sanitizes bad UTF-8
+     *
+     * @param   mixed   $value
+     * @param   int     $options
+     * @param   int     $depth
+     *
+     * @return  string
+     * @throws  JsonEncodeException
+     */
+    public static function sanitize($value, $options = 0, $depth = 512)
+    {
+        return static::encodeAndSanitize($value, $options, $depth, true);
+    }
+
+    /**
+     * {@link json_encode()} wrapper, sanitizes bad UTF-8
+     *
+     * @param   mixed   $value
+     * @param   int     $options
+     * @param   int     $depth
+     * @param   bool    $autoSanitize   Automatically sanitize invalid UTF-8 (if any)
+     *
+     * @return  string
+     * @throws  JsonEncodeException
+     */
+    protected static function encodeAndSanitize($value, $options, $depth, $autoSanitize)
     {
         if (version_compare(phpversion(), '5.5.0', '<')) {
             $encoded = json_encode($value, $options);
         } else {
             $encoded = json_encode($value, $options, $depth);
         }
-        if (json_last_error() !== JSON_ERROR_NONE) {
+
+        switch (json_last_error()) {
+            case JSON_ERROR_NONE:
+                return $encoded;
+
+            /** @noinspection PhpMissingBreakStatementInspection */
+            case JSON_ERROR_UTF8:
+                if ($autoSanitize) {
+                    return static::encode(static::sanitizeUtf8Recursive($value), $options, $depth);
+                }
+                // Fallthrough
+
+            default:
                 throw new JsonEncodeException('%s: %s', static::lastErrorMsg(), var_export($value, true));
         }
-        return $encoded;
     }
 
     /**
@@ -82,4 +124,60 @@ class Json
                 return 'Unknown error';
         }
     }
+
+    /**
+     * Replace bad byte sequences in UTF-8 strings inside the given JSON-encodable structure with question marks
+     *
+     * @param   mixed   $value
+     *
+     * @return  mixed
+     */
+    protected static function sanitizeUtf8Recursive($value)
+    {
+        switch (gettype($value)) {
+            case 'string':
+                return static::sanitizeUtf8String($value);
+
+            case 'array':
+                $sanitized = array();
+
+                foreach ($value as $key => $val) {
+                    if (is_string($key)) {
+                        $key = static::sanitizeUtf8String($key);
+                    }
+
+                    $sanitized[$key] = static::sanitizeUtf8Recursive($val);
+                }
+
+                return $sanitized;
+
+            case 'object':
+                $sanitized = array();
+
+                foreach ($value as $key => $val) {
+                    if (is_string($key)) {
+                        $key = static::sanitizeUtf8String($key);
+                    }
+
+                    $sanitized[$key] = static::sanitizeUtf8Recursive($val);
+                }
+
+                return (object) $sanitized;
+
+            default:
+                return $value;
+        }
+    }
+
+    /**
+     * Replace bad byte sequences in the given UTF-8 string with question marks
+     *
+     * @param   string  $string
+     *
+     * @return  string
+     */
+    protected static function sanitizeUtf8String($string)
+    {
+        return mb_convert_encoding($string, 'UTF-8', 'UTF-8');
+    }
 }

library/Icinga/Web/Announcement/AnnouncementCookie.php

@@ -3,6 +3,7 @@
 
 namespace Icinga\Web\Announcement;
 
+use Icinga\Util\Json;
 use Icinga\Web\Cookie;
 
 /**
@@ -128,7 +129,7 @@ class AnnouncementCookie extends Cookie
      */
     public function getValue()
     {
-        return json_encode(array(
+        return Json::encode(array(
             'acknowledged'  => $this->getAcknowledged(),
             'etag'          => $this->getEtag(),
             'next'          => $this->getNextActive()

library/Icinga/Web/Response/JsonResponse.php

@@ -3,6 +3,7 @@
 
 namespace Icinga\Web\Response;
 
+use Icinga\Util\Json;
 use Zend_Controller_Action_HelperBroker;
 use Icinga\Web\Response;
 
@@ -44,6 +45,13 @@ class JsonResponse extends Response
      */
     protected $encodingOptions = 0;
 
+    /**
+     * Whether to automatically sanitize invalid UTF-8 (if any)
+     *
+     * @var bool
+     */
+    protected $autoSanitize = false;
+
     /**
      * Error message if the API call failed due to a server error
      *
@@ -95,6 +103,30 @@ class JsonResponse extends Response
         return $this;
     }
 
+    /**
+     * Get whether to automatically sanitize invalid UTF-8 (if any)
+     *
+     * @return bool
+     */
+    public function getAutoSanitize()
+    {
+        return $this->autoSanitize;
+    }
+
+    /**
+     * Set whether to automatically sanitize invalid UTF-8 (if any)
+     *
+     * @param   bool    $autoSanitize
+     *
+     * @return  $this
+     */
+    public function setAutoSanitize($autoSanitize = true)
+    {
+        $this->autoSanitize = $autoSanitize;
+
+        return $this;
+    }
+
     /**
      * Get the error message if the API call failed due to a server error
      *
@@ -190,7 +222,9 @@ class JsonResponse extends Response
                 $body['data'] = $this->getSuccessData();
                 break;
         }
-        echo json_encode($body, $this->getEncodingOptions());
+        echo $this->getAutoSanitize()
+            ? Json::sanitize($body, $this->getEncodingOptions())
+            : Json::encode($body, $this->getEncodingOptions());
     }
 
     /**

modules/monitoring/application/clicommands/ListCommand.php

@@ -10,6 +10,7 @@ use Icinga\Cli\Command;
 use Icinga\File\Csv;
 use Icinga\Module\Monitoring\Plugin\PerfdataSet;
 use Exception;
+use Icinga\Util\Json;
 
 /**
  * Icinga monitoring objects
@@ -78,7 +79,7 @@ class ListCommand extends Command
         $query = $query->getQuery();
         switch ($format) {
             case 'json':
-                echo json_encode($query->fetchAll());
+                echo Json::sanitize($query->fetchAll());
                 break;
             case 'csv':
                 Csv::fromQuery($query)->dump();

modules/monitoring/library/Monitoring/Controller.php

@@ -60,7 +60,7 @@ class Controller extends IcingaWebController
                         'Content-Disposition',
                         'inline; filename=' . $this->getRequest()->getActionName() . '.json'
                     )
-                    ->appendBody(Json::encode($query->fetchAll()))
+                    ->appendBody(Json::sanitize($query->fetchAll()))
                     ->sendResponse();
                 exit;
             case 'csv':

modules/monitoring/library/Monitoring/Web/Controller/MonitoredObjectController.php

@@ -153,7 +153,10 @@ abstract class MonitoredObjectController extends Controller
             );
             $groupName = $this->object->getType() . 'groups';
             $payload[$groupName] = $this->object->$groupName;
-            $this->getResponse()->json()->setSuccessData($payload)->sendResponse();
+            $this->getResponse()->json()
+                ->setSuccessData($payload)
+                ->setAutoSanitize()
+                ->sendResponse();
         }
     }
 

modules/monitoring/library/Monitoring/Web/Rest/RestRequest.php

@@ -173,7 +173,7 @@ class RestRequest
     {
         switch ($contentType) {
             case 'application/json':
-                $payload = json_encode($payload);
+                $payload = Json::encode($payload);
                 break;
         }