Fix RootSpanData writer interception on consecutive calls

If there's a cache_slot set, it will bypass the write handler, which very much is not what we want. NULL it to prevent assignment. Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
DataDog · Oct 31, 2024 · f5d48dd · f5d48dd
1 parent 73e9db0
commit f5d48dd
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 0 deletions.
diff --git a/ext/ddtrace.c b/ext/ddtrace.c
@@ -1076,14 +1076,18 @@ static zval *ddtrace_root_span_data_write(zend_object *object, zend_string *memb
         if (Z_TYPE_P(value) == IS_LONG && Z_LVAL_P(value)) {
             span->parent_id = (uint64_t) Z_LVAL_P(value);
             ZVAL_STR(&zv, zend_strpprintf(0, "%" PRIu64, span->parent_id));
+            Z_DELREF(zv); // zend_std_write_property will incref itself
             value = &zv;
         } else {
             span->parent_id = ddtrace_parse_userland_span_id(value);
             if (!span->parent_id) {
+                zval_ptr_dtor(value);
                 ZVAL_EMPTY_STRING(&zv);
+                Z_TRY_DELREF(zv);
                 value = &zv;
             }
         }
+        cache_slot = NULL;
     } else if (zend_string_equals_literal(prop_name, "traceId")) {
         span->trace_id = Z_TYPE_P(value) == IS_STRING ? ddtrace_parse_hex_trace_id(Z_STRVAL_P(value), Z_STRLEN_P(value)) : (ddtrace_trace_id){ 0 };
         if (!span->trace_id.low && !span->trace_id.high) {
@@ -1093,20 +1097,25 @@ static zval *ddtrace_root_span_data_write(zend_object *object, zend_string *memb
             };
             value = &span->property_id;
         }
+        cache_slot = NULL;
     } else if (zend_string_equals_literal(prop_name, "service")) {
         if (ddtrace_span_is_entrypoint_root(&span->span) && !zend_is_identical(&span->property_service, value)) {
             root_span_data_changed = true;
         }
+        cache_slot = NULL;
     } else if (zend_string_equals_literal(prop_name, "env")) {
         if (ddtrace_span_is_entrypoint_root(&span->span) && !zend_is_identical(&span->property_env, value)) {
             root_span_data_changed = true;
         }
+        cache_slot = NULL;
     } else if (zend_string_equals_literal(prop_name, "version")) {
         if (ddtrace_span_is_entrypoint_root(&span->span) && !zend_is_identical(&span->property_version, value)) {
             root_span_data_changed = true;
         }
+        cache_slot = NULL;
     } else if (zend_string_equals_literal(prop_name, "samplingPriority")) {
         span->explicit_sampling_priority = zval_get_long(value) != DDTRACE_PRIORITY_SAMPLING_UNKNOWN;
+        cache_slot = NULL;
     }
 
 #if PHP_VERSION_ID >= 70400

diff --git a/tests/ext/dd_trace_multiple_write.phpt b/tests/ext/dd_trace_multiple_write.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Ensure caching does not bypass the span data write handler
+--FILE--
+<?php
+
+function assign_parent_id($id) {
+    \DDTrace\root_span()->parentId = $id;
+}
+
+assign_parent_id("123");
+var_dump(\DDTrace\root_span()->parentId);
+assign_parent_id("abc");
+var_dump(\DDTrace\root_span()->parentId);
+assign_parent_id(-1);
+var_dump(\DDTrace\root_span()->parentId);
+
+?>
+--EXPECT--
+string(3) "123"
+string(0) ""
+string(20) "18446744073709551615"