Merge pull request #50 from BitOne/GIT-49_pipe_char_in_json

Fixes #49 by escaping control chars

README.md

@@ -148,7 +148,7 @@ Options:
 ### Example
 
 ```bash
-$ bin/analyzer ref-path 0x7f94a1877068 /tmp/php_mem_dump.json
+$ bin/analyzer ref-path -v 0x7f94a1877068 /tmp/php_mem_dump.json
 Found 1 paths
 Path from 0x7f94a1856260
 +--------------------+
@@ -223,6 +223,24 @@ Check the PHP Info output and look for the MemInfo data.
 
 To see the PHP Info output, just create a page calling the `phpinfo();` function, and load it from your browser, or call `php -i` from the command line.
 
+## Why most tests are "skipped"?
+
+While doing a `make test`, some test will need JSON capabilities. But The
+compilation system generates a clean env by removing all configuration
+directives that load extension.
+So if JSON capabilites are packaged as a separate extension (instead of
+being compiled directly in the PHP runtime), the tests will be skipped.
+
+You may run them with the `run-tests.php` generated after the `make test`
+command, by providing the `php` executable:
+
+```bash
+$ TEST_PHP_EXECUTABLE=/usr/bin/php php run-tests.php
+
+```
+In this case, your tests will run with your local PHP configuration,
+including the loading of the JSON extension.
+
 Credits
 -------
 Thanks to Derick Rethans for his inspirational work on the essential XDebug. See http://www.xdebug.org/

extension/php5/meminfo.c

@@ -416,21 +416,33 @@ void meminfo_build_frame_label(char* frame_label, int frame_label_len, zend_exec
 }
 
 /**
- * Escape the \ and " characters for JSON encoding
+ * Escape for JSON encoding
  */
 char * meminfo_escape_for_json(const char *s TSRMLS_DC)
 {
-    int new_str_len;
-    char *s1, *s2;
+    int new_str_len, i;
+    char unescaped_char[2];
+    char escaped_char[7]; // \uxxxx format
+    char *s1, *s2, *s3 = NULL;
 
     s1 = php_str_to_str((char *) s, strlen(s), "\\", 1, "\\\\", 2, &new_str_len);
     s2 = php_str_to_str(s1, strlen(s1), "\"", 1, "\\\"", 2, &new_str_len);
 
-    if (s1) {
-        efree(s1);
+    for (i = 0; i <= 0x1f; i++) {
+        unescaped_char[0] =  (char) i;
+        sprintf(escaped_char, "\\u%04x", i);
+        if (s3) {
+            s2 = s3;
+        }
+        s3 = php_str_to_str(s2, strlen(s2), unescaped_char, 1, escaped_char, 6, &new_str_len);
+        if (s2) {
+            efree(s2);
+        }
     }
 
-    return s2;
+    efree(s1);
+
+    return s3;
 }
 
 /**

extension/php5/tests/bug-git-49_escape_control_character.phpt

@@ -0,0 +1,29 @@
+--TEST--
+Check that control character in array key are properly escaped for json
+--SKIPIF--
+<?php
+    if (!extension_loaded('json')) die('skip json ext not loaded');
+?>
+--FILE--
+<?php
+    $myArray = [
+        "1|\x1f" => "My data"
+    ];
+
+    $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 ok\n";
+    } else {
+        echo "meminfo_dump JSON decode fail\n";
+    }
+
+?>
+--EXPECT--
+meminfo_dump JSON decode ok

extension/php5/tests/dump-check_json_escape.phpt

@@ -0,0 +1,35 @@
+--TEST--
+Check JSON proper escaping (see http://json.org)
+--SKIPIF--
+<?php
+    if (!extension_loaded('json')) die('skip json ext not loaded');
+?>
+--FILE--
+<?php
+    $jsonCharactersToEscape = [
+        '"' => 'quotation mark',
+        '\\' => "reverse solidus",
+        '/' => "solidus"
+    ];
+
+    for ($i = 0; $i <= 0x1f; $i++) {
+        $jsonCharactersToEscape[chr($i)] = "control character $i";
+    }
+
+    $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 ok\n";
+    } else {
+        echo "meminfo_dump JSON decode fail\n";
+    }
+
+?>
+--EXPECT--
+meminfo_dump JSON decode ok

extension/php5/tests/dump-check_json_unicode.phpt

@@ -0,0 +1,29 @@
+--TEST--
+Check JSON proper escaping (see http://json.org)
+--SKIPIF--
+<?php
+    if (!extension_loaded('json')) die('skip json ext not loaded');
+?>
+--FILE--
+<?php
+    $myArrayWithUnicodeKey = [
+        '✈' => 'plane character'
+    ];
+
+    $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 ok\n";
+    } else {
+        echo "meminfo_dump JSON decode fail\n";
+    }
+
+?>
+--EXPECT--
+meminfo_dump JSON decode ok

extension/php7/meminfo.c

@@ -405,21 +405,31 @@ void meminfo_build_frame_label(char* frame_label, int frame_label_len, zend_exec
 }
 
 /**
- * Escape the \ and " characters for JSON encoding
+ * Escape for JSON encoding
  */
 zend_string * meminfo_escape_for_json(const char *s)
 {
-    int new_str_len;
-    zend_string *s1, *s2;
+    int new_str_len, 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);
 
-    if (s1) {
-        zend_string_release(s1);
+    for (i = 0; i <= 0x1f; i++) {
+        unescaped_char[0] =  (char) i;
+        sprintf(escaped_char, "\\u%04x", i);
+        if (s3) {
+            s2 = s3;
+        }
+        s3 = php_str_to_str(ZSTR_VAL(s2), ZSTR_LEN(s2), unescaped_char, 1, escaped_char, 6);
+        zend_string_release(s2);
     }
 
-    return s2;
+    zend_string_release(s1);
+
+    return s3;
 }
 
 /**

extension/php7/tests/bug-git-49_escape_control_character.phpt

@@ -0,0 +1,29 @@
+--TEST--
+Check that control character in array key are properly escaped for json
+--SKIPIF--
+<?php
+    if (!extension_loaded('json')) die('skip json ext not loaded');
+?>
+--FILE--
+<?php
+    $myArray = [
+        "1|\x1f" => "My data"
+    ];
+
+    $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 ok\n";
+    } else {
+        echo "meminfo_dump JSON decode fail\n";
+    }
+
+?>
+--EXPECT--
+meminfo_dump JSON decode ok

extension/php7/tests/dump-check_json_escape.phpt

@@ -0,0 +1,35 @@
+--TEST--
+Check JSON proper escaping (see http://json.org)
+--SKIPIF--
+<?php
+    if (!extension_loaded('json')) die('skip json ext not loaded');
+?>
+--FILE--
+<?php
+    $jsonCharactersToEscape = [
+        '"' => 'quotation mark',
+        '\\' => "reverse solidus",
+        '/' => "solidus"
+    ];
+
+    for ($i = 0; $i <= 0x1f; $i++) {
+        $jsonCharactersToEscape[chr($i)] = "control character $i";
+    }
+
+    $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 ok\n";
+    } else {
+        echo "meminfo_dump JSON decode fail\n";
+    }
+
+?>
+--EXPECT--
+meminfo_dump JSON decode ok

extension/php7/tests/dump-check_json_unicode.phpt

@@ -0,0 +1,29 @@
+--TEST--
+Check JSON proper escaping (see http://json.org)
+--SKIPIF--
+<?php
+    if (!extension_loaded('json')) die('skip json ext not loaded');
+?>
+--FILE--
+<?php
+    $myArrayWithUnicodeKey = [
+        '✈' => 'plane character'
+    ];
+
+    $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 ok\n";
+    } else {
+        echo "meminfo_dump JSON decode fail\n";
+    }
+
+?>
+--EXPECT--
+meminfo_dump JSON decode ok