Address review comments

appsec/src/extension/commands/request_shutdown.c

@@ -14,8 +14,21 @@
 #include "request_shutdown_arginfo.h"
 #include "zend_exceptions.h"
 #include <SAPI.h>
+#include <dlfcn.h>
 #include <ext/json/php_json.h>
 
+#if PHP_VERSION_ID < 70200
+typedef void (*json_decode_ex_t)(zval *return_value, char *str, size_t str_len,
+    zend_long options, zend_long depth);
+#elif PHP_VERSION_ID < 80200
+typedef int (*json_decode_ex_t)(zval *return_value, const char *str,
+    size_t str_len, zend_long options, zend_long depth);
+#else
+typedef zend_result (*json_decode_ex_t)(zval *return_value, const char *str,
+    size_t str_len, zend_long options, zend_long depth);
+#endif
+static json_decode_ex_t _json_decode_ex;
+
 static dd_result _request_pack(mpack_writer_t *nonnull w, void *nonnull ctx);
 static void _pack_headers_no_cookies_llist(
     mpack_writer_t *nonnull w, zend_llist *nonnull hl);
@@ -306,7 +319,11 @@ static zval _convert_json(char *nonnull entity, size_t entity_len)
 {
     zval zv;
     ZVAL_NULL(&zv);
-#define MAX_DEPTH 12
+    if (!_json_decode_ex) {
+        return zv;
+    }
+
+#define MAX_DEPTH 30
     php_json_decode_ex(
         &zv, entity, entity_len, PHP_JSON_OBJECT_AS_ARRAY, MAX_DEPTH);
     if (Z_TYPE(zv) == IS_NULL) {
@@ -345,6 +362,33 @@ static bool _assume_utf8(const char *ct, size_t ct_len)
     return true;
 }
 
+static zend_array *_transform_attr_keys(const zval *orig)
+{
+    // append @ to keys
+    zend_array *new_arr = zend_new_array(zend_array_count(Z_ARR_P(orig)));
+    zend_string *key;
+    zval *val;
+    ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARR_P(orig), key, val)
+    {
+        if (!key) {
+            // attribute names can't be only numbers anyway
+            continue;
+        }
+        if (Z_TYPE_P(val) != IS_STRING) {
+            continue;
+        }
+        char *new_key = safe_emalloc(ZSTR_LEN(key), 1, 2);
+        char *wp = new_key;
+        *wp++ = '@';
+        memcpy(wp, ZSTR_VAL(key), ZSTR_LEN(key) + 1);
+        zval_addref_p(val);
+        zend_hash_str_add_new(new_arr, new_key, ZSTR_LEN(key) + 1, val);
+        efree(new_key);
+    }
+    ZEND_HASH_FOREACH_END();
+    return new_arr;
+}
+
 static zval _convert_xml_impl(const char *nonnull entity, size_t entity_len,
     const char *content_type, size_t content_type_len)
 {
@@ -434,7 +478,7 @@ static zval _convert_xml_impl(const char *nonnull entity, size_t entity_len,
 
     // now transform the the result
     // each tag is encoded as a singleton map:
-    // <tag name>: {content: [...], attributes: {...})
+    // <tag name>: [{@attr1: "...", ...}, "text...", {further tags...}])
     // text is encoded as string
     zend_array *root = zend_new_array(1);
     zend_array *cur = root; // non-owning
@@ -489,42 +533,33 @@ static zval _convert_xml_impl(const char *nonnull entity, size_t entity_len,
             ZVAL_ARR(&celem_zv, celem);
             zend_hash_next_index_insert(cur, &celem_zv);
 
-            // map with keys content and attributes
-            zend_array *celem_val = zend_new_array(attr_zv ? 2 : 1);
+            // array with content and attributes
+            zend_array *celem_val =
+                zend_new_array(attr_zv ? 2 : 1 /* estimate only */);
             {
                 zval celem_val_zv;
                 ZVAL_ARR(&celem_val_zv, celem_val);
                 zend_hash_add_new(celem, Z_STR_P(tag_zv), &celem_val_zv);
             }
 
-            zend_array *content = NULL;
-            if (type == open || value_zv) {
-                content = zend_new_array(1);
-                {
-                    zval content_zv;
-                    ZVAL_ARR(&content_zv, content);
-                    zend_hash_str_add_new(celem_val, "content",
-                        sizeof("content") - 1, &content_zv);
-                }
-                if (value_zv) {
-                    zval_addref_p(value_zv);
-                    zend_hash_next_index_insert(content, value_zv);
-                }
+            if (attr_zv) {
+                zend_array *new_attr = _transform_attr_keys(attr_zv);
+                zval new_attr_zv;
+                ZVAL_ARR(&new_attr_zv, new_attr);
+                zend_hash_next_index_insert(celem_val, &new_attr_zv);
             }
 
-            if (attr_zv) {
-                zval_addref_p(attr_zv);
-                zend_hash_str_add_new(
-                    celem_val, "attributes", sizeof("attributes") - 1, attr_zv);
+            if (value_zv) {
+                zval_addref_p(value_zv);
+                zend_hash_next_index_insert(celem_val, value_zv);
             }
 
             if (type == open) {
                 // stash cur, cur = content
                 zval cur_zv;
                 ZVAL_ARR(&cur_zv, cur);
                 zend_hash_next_index_insert(stack, &cur_zv);
-                assert(content != NULL);
-                cur = content;
+                cur = celem_val;
             }
         } else if (type == cdata) {
             zval *value_zv = zend_hash_str_find(val, LSTRARG("value"));
@@ -609,6 +644,22 @@ PHP_FUNCTION(datadog_appsec_testing_convert_xml)
 
 void dd_request_shutdown_startup()
 {
+#if PHP_VERSION_ID < 80000
+    void *handle = dlopen(NULL, RTLD_NOW | RTLD_GLOBAL);
+    if (handle == NULL) {
+        // NOLINTNEXTLINE(concurrency-mt-unsafe)
+        mlog(dd_log_error, "Failed load process symbols: %s", dlerror());
+    } else {
+        _json_decode_ex = (json_decode_ex_t)dlsym(handle, "php_json_decode_ex");
+        if (!_json_decode_ex) {
+            mlog(dd_log_warning, "Failed to load php_json_decode_ex: %s",
+                dlerror()); // NOLINT(concurrency-mt-unsafe)
+        }
+        dlclose(handle);
+    }
+#else
+    _json_decode_ex = php_json_decode_ex;
+#endif
     if (get_global_DD_APPSEC_TESTING()) {
         dd_phpobj_reg_funcs(ext_functions);
     }

appsec/src/extension/ddappsec.c

@@ -166,6 +166,7 @@ static PHP_GINIT_FUNCTION(ddappsec)
 
 static PHP_GSHUTDOWN_FUNCTION(ddappsec)
 {
+    dd_entity_body_gshutdown();
     dd_helper_gshutdown();
     // delay log shutdown until the last possible moment, so that TSRM
     // destructors can run with logging
@@ -231,7 +232,6 @@ static PHP_MSHUTDOWN_FUNCTION(ddappsec)
     // no other thread is running now. reset config to global config only.
     runtime_config_first_init = false;
 
-    dd_entity_body_shutdown();
     dd_tags_shutdown();
     dd_user_tracking_shutdown();
     dd_trace_shutdown();

appsec/src/extension/entity_body.c

@@ -15,10 +15,6 @@ static typeof(zend_write(NULL, 0)) _dd_save_output_zend_write(
 ZEND_TLS zend_string *_buffer;
 ZEND_TLS size_t _buffer_size;
 
-// we need to keep track of all buffers so we can free them on shutdown
-// this is in order to avoid having memory leaks reported
-static HashTable _all_buffers;
-
 #define DEFAULT_MAX_BUF_SIZE (1024 * 512UL)
 
 static typeof(zend_write) orig_zend_write;
@@ -27,25 +23,14 @@ void dd_entity_body_startup()
 {
     orig_zend_write = zend_write;
     zend_write = _dd_save_output_zend_write;
-    zend_hash_init(&_all_buffers, 0, NULL, NULL, 1);
 }
 
-void dd_entity_body_shutdown()
+void dd_entity_body_gshutdown()
 {
-    zend_write = orig_zend_write;
-    zend_string *key_s = NULL;
-    zend_ulong key_i;
-    ZEND_HASH_FOREACH_KEY(&_all_buffers, key_i, key_s)
-    {
-        UNUSED(key_i);
-        assert(key_s != NULL);
-        zend_string *s;
-        memcpy(&s, ZSTR_VAL(key_s), sizeof(s)); // NOLINT
-        assert(ZSTR_LEN(key_s) == sizeof(s));   // NOLINT
-        zend_string_release(s);
+    if (_buffer) {
+        zend_string_release(_buffer);
+        _buffer = NULL;
     }
-    ZEND_HASH_FOREACH_END();
-    zend_hash_destroy(&_all_buffers);
 }
 
 static typeof(zend_write(NULL, 0)) _dd_save_output_zend_write(
@@ -72,13 +57,8 @@ void dd_entity_body_activate()
     if (desired_bufsize != _buffer_size) {
         if (_buffer != NULL) {
             zend_string_release(_buffer);
-            zend_hash_str_del(
-                &_all_buffers, (void *)&_buffer, sizeof(_buffer)); // NOLINT
         }
         _buffer = zend_string_alloc(desired_bufsize + /* NUL */ 1, 1);
-        // NOLINTNEXTLINE
-        zend_hash_str_add(&_all_buffers, (void *)&_buffer, sizeof(_buffer),
-            &(zval){.u1.type_info = IS_NULL}); // value is irrelevant
         _buffer_size = desired_bufsize;
     }
 

appsec/src/extension/entity_body.h

@@ -10,7 +10,7 @@
 #include <stdbool.h>
 
 void dd_entity_body_startup(void);
-void dd_entity_body_shutdown(void);
+void dd_entity_body_gshutdown(void);
 void dd_entity_body_activate(void);
 zend_string *nonnull dd_request_body_buffered(size_t limit);
 zend_string *nonnull dd_response_body_buffered(void);

appsec/src/extension/request_lifecycle.c

@@ -483,8 +483,10 @@ static zend_string *nullable _get_entity_as_string(zval *rbe_zv)
 
     // TODO: support non-seekable streams. Needs replacing the stream
     if (stream->flags & PHP_STREAM_FLAG_NO_SEEK) {
-        mlog(dd_log_info, "Response body entity is a stream, but it is "
-                          "not seekable; ignoring");
+        __auto_type lvl =
+            get_global_DD_APPSEC_TESTING() ? dd_log_info : dd_log_debug;
+        mlog(lvl, "Response body entity is a stream, but it is "
+                  "not seekable; ignoring");
         return NULL;
     }
 
@@ -525,7 +527,9 @@ static zend_string *nullable _get_entity_as_string(zval *rbe_zv)
 
     size_t effective_size = stream_size - (size_t)start_pos;
     if (effective_size >= (size_t)get_DD_APPSEC_MAX_BODY_BUFF_SIZE()) {
-        mlog(dd_log_info,
+        __auto_type lvl =
+            get_global_DD_APPSEC_TESTING() ? dd_log_info : dd_log_debug;
+        mlog(lvl,
             "Response body entity is larger than %zu bytes (got %zu); ignoring",
             max_size, effective_size);
         return NULL;

appsec/tests/extension/convert_json_max_depth.phpt

@@ -0,0 +1,36 @@
+--TEST--
+_convert_json function (exceed max depth)
+--FILE--
+<?php
+function generateNestedArray($depth) {
+    $array = [];
+    $current = &$array;
+    for ($i = 1; $i <= $depth; $i++) {
+        $current[$i] = [];
+        $current = &$current[$i];
+    }
+    return $array;
+}
+
+function t($depth) {
+    $j = generateNestedArray($depth);
+    $data = json_encode($j);
+    echo "Original ($depth):\n";
+    var_dump($data);
+
+
+    $result = datadog\appsec\testing\convert_json($data);
+    echo "After transformation ($depth):\n";
+    echo(json_encode($result)), "\n";
+}
+t(28);
+t(29);
+--EXPECT--
+Original (28):
+string(189) "{"1":{"2":{"3":{"4":{"5":{"6":{"7":{"8":{"9":{"10":{"11":{"12":{"13":{"14":{"15":{"16":{"17":{"18":{"19":{"20":{"21":{"22":{"23":{"24":{"25":{"26":{"27":{"28":[]}}}}}}}}}}}}}}}}}}}}}}}}}}}}"
+After transformation (28):
+{"1":{"2":{"3":{"4":{"5":{"6":{"7":{"8":{"9":{"10":{"11":{"12":{"13":{"14":{"15":{"16":{"17":{"18":{"19":{"20":{"21":{"22":{"23":{"24":{"25":{"26":{"27":{"28":[]}}}}}}}}}}}}}}}}}}}}}}}}}}}}
+Original (29):
+string(196) "{"1":{"2":{"3":{"4":{"5":{"6":{"7":{"8":{"9":{"10":{"11":{"12":{"13":{"14":{"15":{"16":{"17":{"18":{"19":{"20":{"21":{"22":{"23":{"24":{"25":{"26":{"27":{"28":{"29":[]}}}}}}}}}}}}}}}}}}}}}}}}}}}}}"
+After transformation (29):
+null