Fix invalid JSON dump when a string holds non UTF-8 bytes

extension/meminfo.c

@@ -6,8 +6,9 @@
 #include "php_meminfo.h"
 
 #include "ext/standard/info.h"
-#include "ext/standard/php_string.h"
+#include "ext/standard/html.h"
 
+#include "zend_smart_str.h"
 #include "zend_extensions.h"
 #include "zend_exceptions.h"
 #include "Zend/zend_compile.h"
@@ -526,30 +527,52 @@ void meminfo_build_frame_label(char* frame_label, int frame_label_len, zend_exec
 
 /**
  * Escape for JSON encoding
+ *
+ * Escapes the two characters JSON forbids raw inside a string (double quote and
+ * backslash) plus every control character, and substitutes any byte that is not
+ * part of a valid UTF-8 sequence with U+FFFD.
+ *
+ * PHP strings are byte arrays with no encoding guarantee, so an array key, a
+ * property name or a class name can hold arbitrary binary data. Writing such a
+ * byte out raw makes the whole dump undecodable ("Malformed UTF-8 characters,
+ * possibly incorrectly encoded"), which is why invalid sequences are replaced
+ * rather than copied through.
  */
 zend_string * meminfo_escape_for_json(const char *s)
 {
-    int i;
-    char unescaped_char[2];
-    char escaped_char[7]; // \uxxxx format
-    zend_string *s1, *s2, *s3 = NULL;
-
-    s1 = php_str_to_str((char *) s, strlen(s), "\\", 1, "\\\\", 2);
-    s2 = php_str_to_str(ZSTR_VAL(s1), ZSTR_LEN(s1), "\"", 1, "\\\"", 2);
-
-    for (i = 0; i <= 0x1f; i++) {
-        unescaped_char[0] =  (char) i;
-        sprintf(escaped_char, "\\u%04x", i);
-        if (s3) {
-            s2 = s3;
+    size_t length = strlen(s);
+    size_t position = 0;
+    smart_str output = {0};
+
+    while (position < length) {
+        size_t sequence_start = position;
+        int status = SUCCESS;
+        unsigned int code_point;
+
+        // Advances position past the sequence, and by one byte on invalid input
+        code_point = php_next_utf8_char((const unsigned char *) s, length, &position, &status);
+
+        if (status != SUCCESS) {
+            smart_str_appendl(&output, MEMINFO_JSON_REPLACEMENT_CHARACTER,
+                sizeof(MEMINFO_JSON_REPLACEMENT_CHARACTER) - 1);
+        } else if (code_point == '"' || code_point == '\\') {
+            smart_str_appendc(&output, '\\');
+            smart_str_appendc(&output, (char) code_point);
+        } else if (code_point <= MEMINFO_JSON_LAST_CONTROL_CHARACTER) {
+            char escaped_char[MEMINFO_JSON_CONTROL_ESCAPE_SIZE];
+
+            snprintf(escaped_char, sizeof(escaped_char), MEMINFO_JSON_CONTROL_ESCAPE_FORMAT, code_point);
+            smart_str_appendl(&output, escaped_char, sizeof(escaped_char) - 1);
+        } else {
+            // Valid UTF-8 is legal as-is in a JSON string, keep the original bytes
+            smart_str_appendl(&output, s + sequence_start, position - sequence_start);
         }
-        s3 = php_str_to_str(ZSTR_VAL(s2), ZSTR_LEN(s2), unescaped_char, 1, escaped_char, 6);
-        zend_string_release(s2);
     }
 
-    zend_string_release(s1);
+    smart_str_0(&output);
 
-    return s3;
+    // smart_str allocates lazily, so an empty input leaves nothing behind
+    return output.s ? output.s : ZSTR_EMPTY_ALLOC();
 }
 
 #ifdef COMPILE_DL_MEMINFO

extension/php_meminfo.h

@@ -10,6 +10,13 @@ extern zend_module_entry meminfo_module_entry;
 #define MEMINFO_COPYRIGHT  "Copyright (c) 2010-2021 by Benoit Jacquemont & contributors"
 #define MEMINFO_COPYRIGHT_SHORT "Copyright (c) 2010-2021"
 
+// JSON string escaping, see meminfo_escape_for_json()
+#define MEMINFO_JSON_LAST_CONTROL_CHARACTER 0x1f
+#define MEMINFO_JSON_CONTROL_ESCAPE_FORMAT "\\u%04x"
+#define MEMINFO_JSON_CONTROL_ESCAPE_SIZE sizeof("\\uffff")
+// U+FFFD REPLACEMENT CHARACTER, same substitution as JSON_INVALID_UTF8_SUBSTITUTE
+#define MEMINFO_JSON_REPLACEMENT_CHARACTER "\\ufffd"
+
 PHP_FUNCTION(meminfo_dump);
 
 zend_ulong   meminfo_get_element_size(zval* z);

extension/tests/bug-github-124_invalid_utf8_in_array_key.phpt

@@ -0,0 +1,45 @@
+--TEST--
+Check that bytes that are not valid UTF-8 don't break the JSON dump
+--SKIPIF--
+<?php
+    if (!extension_loaded('json')) die('skip json ext not loaded');
+?>
+--FILE--
+<?php
+    $myArray = [
+        // Latin-1 copyright sign: a lone continuation byte, so invalid UTF-8
+        "\xA9" => 'latin-1 copyright sign',
+        // U+2708 AIRPLANE: valid UTF-8, must be left untouched
+        "\xE2\x9C\x88" => 'valid utf-8 airplane',
+    ];
+
+    $dump = fopen('php://memory', 'rw');
+
+    meminfo_dump($dump);
+
+    rewind($dump);
+    $meminfoData = json_decode(stream_get_contents($dump), true);
+    fclose($dump);
+
+    if (!is_array($meminfoData)) {
+        echo "meminfo_dump JSON decode fail: ", json_last_error_msg(), "\n";
+        return;
+    }
+
+    echo "meminfo_dump JSON decode ok\n";
+
+    $dumpedKeys = [];
+    foreach ($meminfoData['items'] as $item) {
+        if (isset($item['children'])) {
+            $dumpedKeys += $item['children'];
+        }
+    }
+
+    // U+FFFD REPLACEMENT CHARACTER: the invalid byte must be substituted, not emitted raw
+    echo "invalid byte replaced: ", (isset($dumpedKeys["\xEF\xBF\xBD"]) ? 'yes' : 'no'), "\n";
+    echo "valid utf-8 preserved: ", (isset($dumpedKeys["\xE2\x9C\x88"]) ? 'yes' : 'no'), "\n";
+?>
+--EXPECT--
+meminfo_dump JSON decode ok
+invalid byte replaced: yes
+valid utf-8 preserved: yes