Misc changes coming from PR

appsec/src/extension/backtrace.c

@@ -25,7 +25,8 @@ static zend_string *_frame_file;
 static zend_string *_id_key;
 
 bool php_backtrace_frame_to_datadog_backtrace_frame( // NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
-    zval *php_backtrace_frame, zval *datadog_backtrace_frame, zend_ulong index)
+    zval *nonnull php_backtrace_frame, zval *nonnull datadog_backtrace_frame,
+    zend_ulong index)
 {
     if (Z_TYPE_P(php_backtrace_frame) != IS_ARRAY) {
         return false;
@@ -61,7 +62,7 @@ bool php_backtrace_frame_to_datadog_backtrace_frame( // NOLINTNEXTLINE(bugprone-
 
 void php_backtrace_to_datadog_backtrace(
     // NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
-    zval *php_backtrace, zval *datadog_backtrace)
+    zval *nonnull php_backtrace, zval *nonnull datadog_backtrace)
 {
     if (Z_TYPE_P(php_backtrace) != IS_ARRAY) {
         return;
@@ -72,7 +73,8 @@ void php_backtrace_to_datadog_backtrace(
 
     unsigned int top = frames_on_stack;
     unsigned int bottom = 0;
-    if (get_global_DD_APPSEC_MAX_STACK_TRACE_DEPTH() < frames_on_stack) {
+    if (get_global_DD_APPSEC_MAX_STACK_TRACE_DEPTH() != 0 &&
+        get_global_DD_APPSEC_MAX_STACK_TRACE_DEPTH() < frames_on_stack) {
         top = (unsigned int)round(
             (double)get_global_DD_APPSEC_MAX_STACK_TRACE_DEPTH() *
             STACK_DEFAULT_TOP_PERCENTAGE);
@@ -121,11 +123,11 @@ void php_backtrace_to_datadog_backtrace(
     }
 }
 
-void generate_backtrace(zend_string *id, zval *dd_backtrace)
+void dd_generate_backtrace(zend_string *nullable id, zval *nonnull dd_backtrace)
 {
     array_init(dd_backtrace);
 
-    if (!get_global_DD_APPSEC_STACK_TRACE_ENABLED() || !id) {
+    if (!id) {
         return;
     }
 
@@ -153,23 +155,20 @@ static PHP_FUNCTION(datadog_appsec_testing_generate_backtrace)
         RETURN_FALSE;
     }
 
-    generate_backtrace(id, return_value);
+    dd_generate_backtrace(id, return_value);
 }
 
-zval *dd_hash_find_or_new(HashTable *ht, zend_string *key)
+bool dd_report_exploit_backtrace(zend_string *nullable id)
 {
-    zval *result = zend_hash_find(ht, key);
-
-    if (!result) {
-        zval new_zv;
-        result = zend_hash_add(ht, key, &new_zv);
+    if (!get_global_DD_APPSEC_STACK_TRACE_ENABLED()) {
+        return false;
     }
 
-    return result;
-}
+    if (!id) {
+        mlog(dd_log_warning,
+            "Backtrace can not be generated because id is missing");
+    }
 
-bool report_backtrace(zend_string *id)
-{
     zend_object *span = dd_trace_get_active_root_span();
     if (!span) {
         if (!get_global_DD_APPSEC_TESTING()) {
@@ -200,32 +199,34 @@ bool report_backtrace(zend_string *id)
         array_init(exploit);
     }
 
-    if (zend_array_count(Z_ARR_P(exploit)) ==
-        get_global_DD_APPSEC_MAX_STACK_TRACES()) {
+    unsigned int limit = get_global_DD_APPSEC_MAX_STACK_TRACES();
+    if (limit != 0 && zend_array_count(Z_ARR_P(exploit)) == limit) {
+        mlog(dd_log_debug,
+            "Stacktrace not generated due to limit "
+            "D_APPSEC_MAX_STACK_TRACES(%d) has been reached",
+            limit);
         return false;
     }
 
     zval backtrace;
-    generate_backtrace(id, &backtrace);
+    dd_generate_backtrace(id, &backtrace);
 
     if (zend_hash_next_index_insert_new(Z_ARRVAL_P(exploit), &backtrace) ==
         NULL) {
         return false;
     }
 
-    zend_hash_add(Z_ARRVAL_P(meta_struct), _dd_stack_key, dd_stack);
-
     return true;
 }
 
-static PHP_FUNCTION(datadog_appsec_testing_report_backtrace)
+static PHP_FUNCTION(datadog_appsec_testing_report_exploit_backtrace)
 {
     zend_string *id = NULL;
     if (zend_parse_parameters(ZEND_NUM_ARGS(), "S", &id) != SUCCESS) {
         RETURN_FALSE;
     }
 
-    if (report_backtrace(id)) {
+    if (dd_report_exploit_backtrace(id)) {
         RETURN_TRUE;
     }
 
@@ -245,7 +246,7 @@ ZEND_END_ARG_INFO()
 // clang-format off
 static const zend_function_entry testing_functions[] = {
     ZEND_RAW_FENTRY(DD_TESTING_NS "generate_backtrace", PHP_FN(datadog_appsec_testing_generate_backtrace), void_ret_array_arginfo,0)
-    ZEND_RAW_FENTRY(DD_TESTING_NS "report_backtrace", PHP_FN(datadog_appsec_testing_report_backtrace), void_ret_bool_arginfo, 0)
+    ZEND_RAW_FENTRY(DD_TESTING_NS "report_exploit_backtrace", PHP_FN(datadog_appsec_testing_report_exploit_backtrace), void_ret_bool_arginfo, 0)
     PHP_FE_END
 };
 // clang-format on
@@ -261,19 +262,15 @@ static void _register_testing_objects()
 
 void dd_backtrace_startup()
 {
-    _frames_key = zend_string_init_interned("frames", sizeof("frames") - 1, 1);
-    _language_key =
-        zend_string_init_interned("language", sizeof("language") - 1, 1);
-    _php_value = zend_string_init_interned("php", sizeof("php") - 1, 1);
-    _exploit_key =
-        zend_string_init_interned("exploit", sizeof("exploit") - 1, 1);
-    _dd_stack_key =
-        zend_string_init_interned("_dd.stack", sizeof("_dd.stack") - 1, 1);
-    _frame_line = zend_string_init_interned("line", sizeof("line") - 1, 1);
-    _frame_function =
-        zend_string_init_interned("function", sizeof("function") - 1, 1);
-    _frame_file = zend_string_init_interned("file", sizeof("file") - 1, 1);
-    _id_key = zend_string_init_interned("id", sizeof("id") - 1, 1);
+    _frames_key = zend_string_init_interned(LSTRARG("frames"), 1);
+    _language_key = zend_string_init_interned(LSTRARG("language"), 1);
+    _php_value = zend_string_init_interned(LSTRARG("php"), 1);
+    _exploit_key = zend_string_init_interned(LSTRARG("exploit"), 1);
+    _dd_stack_key = zend_string_init_interned(LSTRARG("_dd.stack"), 1);
+    _frame_line = zend_string_init_interned(LSTRARG("line"), 1);
+    _frame_function = zend_string_init_interned(LSTRARG("function"), 1);
+    _frame_file = zend_string_init_interned(LSTRARG("file"), 1);
+    _id_key = zend_string_init_interned(LSTRARG("id"), 1);
 #ifdef TESTING
     _register_testing_objects();
 #endif

appsec/src/extension/backtrace.h

@@ -9,11 +9,13 @@
 #include <SAPI.h>
 #include <Zend/zend_builtin_functions.h>
 #include <php.h>
-
 #include <stdbool.h>
 
+#include "attributes.h"
+
 void dd_backtrace_startup();
-void generate_backtrace(zend_string *id, zval *dd_backtrace);
-bool report_backtrace(zend_string *id);
+void dd_generate_backtrace(
+    zend_string *nullable id, zval *nonnull dd_backtrace);
+bool dd_report_exploit_backtrace(zend_string *nullable id);
 
 #endif // BACKTRACE_H

appsec/src/extension/commands_helpers.c

@@ -431,9 +431,9 @@ static void _command_process_stack_trace_parameters(mpack_node_t root)
             zend_string *id = NULL;
             size_t id_len = mpack_node_strlen(value);
             id = zend_string_init(mpack_node_str(value), id_len, 0);
-            report_backtrace(id);
+            dd_report_exploit_backtrace(id);
             zend_string_release(id);
-            --expected_nodes;
+            break;
         }
     }
 }

appsec/src/extension/ddappsec.c

@@ -6,7 +6,6 @@
 #include <SAPI.h>
 #include <Zend/zend_extensions.h>
 #include <ext/standard/info.h>
-#include <ext/standard/php_var.h>
 #include <php.h>
 
 // for open(2)

appsec/src/extension/ddtrace.c

@@ -62,7 +62,7 @@ static void dd_trace_load_symbols(void)
     if (_ddtrace_close_all_spans_and_flush == NULL && !testing) {
         mlog(dd_log_error,
             // NOLINTNEXTLINE(concurrency-mt-unsafe)
-            "Failed to load ddtrace _close_all_spans_and_flush: %s", dlerror());
+            "Failed to load ddtrace_close_all_spans_and_flush: %s", dlerror());
     }
 
     _ddtrace_get_root_span = dlsym(handle, "ddtrace_get_root_span");
@@ -293,27 +293,6 @@ zval *nullable dd_trace_span_get_meta_struct(zend_object *nonnull zobj)
     return _get_span_modifiable_array_property(zobj, _meta_struct_propname);
 }
 
-void add_entry_to_meta_struct(zend_string *nonnull key, zval *nonnull value)
-{
-    zend_object *span = dd_trace_get_active_root_span();
-    if (!span) {
-        return;
-    }
-    zval *meta_struct = dd_trace_span_get_meta_struct(span);
-    if (!meta_struct) {
-        if (!get_global_DD_APPSEC_TESTING()) {
-            mlog(dd_log_warning, "Failed to retrieve root span meta_struct");
-        }
-        zval_ptr_dtor(value);
-        return;
-    }
-
-    if (zend_hash_add(Z_ARRVAL_P(meta_struct), key, value) == NULL) {
-        zval_ptr_dtor(value);
-        return;
-    }
-}
-
 // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers)
 zend_string *nullable dd_trace_get_formatted_runtime_id(bool persistent)
 {

appsec/src/extension/ddtrace.h

@@ -45,8 +45,6 @@ bool dd_trace_span_add_tag_str(zend_object *nonnull zobj,
     const char *nonnull tag, size_t tag_len, const char *nonnull value,
     size_t value_len);
 
-void add_entry_to_meta_struct(zend_string *nonnull key, zval *nonnull value);
-
 // Flush the tracer spans, can be used on RINIT
 void dd_trace_close_all_spans_and_flush(void);
 

appsec/src/extension/php_helpers.c

@@ -115,3 +115,15 @@ zend_string *nullable dd_php_get_string_elem(
 
     return Z_STR_P(zresult);
 }
+
+zval *dd_hash_find_or_new(HashTable *ht, zend_string *key)
+{
+    zval *result = zend_hash_find(ht, key);
+
+    if (!result) {
+        zval new_zv;
+        result = zend_hash_add(ht, key, &new_zv);
+    }
+
+    return result;
+}

appsec/src/extension/php_helpers.h

@@ -48,3 +48,4 @@ zend_string *nullable dd_php_get_string_elem(
     const zend_array *nullable arr, zend_string *nonnull zstr);
 zend_string *nullable dd_php_get_string_elem_cstr(
     const zend_array *nullable arr, const char *nonnull name, size_t len);
+zval *dd_hash_find_or_new(HashTable *ht, zend_string *key);

appsec/tests/extension/generate_backtrace_02.phpt

@@ -1,9 +1,9 @@
 --TEST--
-Generate backtrace is not generated when disabled
+Number of frames can be configured with DD_APPSEC_MAX_STACK_TRACE_DEPTH
 --INI--
 extension=ddtrace.so
 --ENV--
-DD_APPSEC_STACK_TRACE_ENABLED=false
+DD_APPSEC_MAX_STACK_TRACE_DEPTH=1
 --FILE--
 <?php
 
@@ -23,5 +23,23 @@ one("foo");
 
 ?>
 --EXPECTF--
-array(0) {
+array(3) {
+  ["language"]=>
+  string(3) "php"
+  ["id"]=>
+  string(7) "some id"
+  ["frames"]=>
+  array(1) {
+    [0]=>
+    array(4) {
+      ["line"]=>
+      int(15)
+      ["function"]=>
+      string(3) "one"
+      ["file"]=>
+      string(25) "generate_backtrace_02.php"
+      ["id"]=>
+      int(1)
+    }
+  }
 }

appsec/tests/extension/generate_backtrace_03.phpt

@@ -1,45 +1,24 @@
 --TEST--
-Number of frames can be configured with DD_APPSEC_MAX_STACK_TRACE_DEPTH
+By default DD_APPSEC_MAX_STACK_TRACE_DEPTH is 32
 --INI--
 extension=ddtrace.so
---ENV--
-DD_APPSEC_MAX_STACK_TRACE_DEPTH=1
 --FILE--
 <?php
 
 use function datadog\appsec\testing\generate_backtrace;
 
-function two($param01, $param02)
+function recursive_function($limit)
 {
-    var_dump(generate_backtrace("some id"));
-}
+    if (--$limit == 0) {
+        var_dump(count(generate_backtrace("some id")["frames"]));
+        return;
+    }
 
-function one($param01)
-{
-    two($param01, "other");
+    recursive_function($limit);
 }
 
-one("foo");
+recursive_function(40);
 
 ?>
 --EXPECTF--
-array(3) {
-  ["language"]=>
-  string(3) "php"
-  ["id"]=>
-  string(7) "some id"
-  ["frames"]=>
-  array(1) {
-    [0]=>
-    array(4) {
-      ["line"]=>
-      int(15)
-      ["function"]=>
-      string(3) "one"
-      ["file"]=>
-      string(25) "generate_backtrace_03.php"
-      ["id"]=>
-      int(1)
-    }
-  }
-}
+int(32)