diff --git a/extension/meminfo.c b/extension/meminfo.c index b91b5b5..c76b15f 100644 --- a/extension/meminfo.c +++ b/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 diff --git a/extension/php_meminfo.h b/extension/php_meminfo.h index 5b3e399..7a419ae 100644 --- a/extension/php_meminfo.h +++ b/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); diff --git a/extension/tests/bug-github-124_invalid_utf8_in_array_key.phpt b/extension/tests/bug-github-124_invalid_utf8_in_array_key.phpt new file mode 100644 index 0000000..9fb0bce --- /dev/null +++ b/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-- + +--FILE-- + '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