From 2933281a8bbc33c7d699537a2a3637ec13ce1f1c Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Thu, 31 Oct 2024 16:22:04 -0400 Subject: [PATCH 1/8] Adding test to base behavior from --- .../dd_trace_serialize_header_to_meta.phpt | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 tests/ext/dd_trace_serialize_header_to_meta.phpt diff --git a/tests/ext/dd_trace_serialize_header_to_meta.phpt b/tests/ext/dd_trace_serialize_header_to_meta.phpt new file mode 100644 index 0000000000..9146210e58 --- /dev/null +++ b/tests/ext/dd_trace_serialize_header_to_meta.phpt @@ -0,0 +1,20 @@ +--TEST-- +Headers values are mapped to expected tag key +--ENV-- +HTTP_CONTENT_TYPE=text/plain +HTTP_CUSTOM_HEADER=custom-header-value +DD_TRACE_GENERATE_ROOT_SPAN=0 +DD_TRACE_HEADER_TAGS=Content-Type,Custom-Header:custom-header-key +--GET-- +application_key=123 +--FILE-- + +--EXPECT-- +string(10) "text/plain" +string(13) "custom-header-value" From a1939a29172c5d6a0e9bbc2406729a8ca05a552b Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Mon, 4 Nov 2024 19:17:46 -0500 Subject: [PATCH 2/8] Using MAP_LOWERCASE and updating current zai_config_decode_map method --- ext/configuration.h | 4 +- ext/serializer.c | 18 +++-- tests/Unit/ConfigurationTest.php | 7 +- .../dd_trace_serialize_header_to_meta.phpt | 6 +- .../config/config_decode.c | 76 ++++++++++++++----- .../config/config_decode.h | 1 + 6 files changed, 78 insertions(+), 34 deletions(-) diff --git a/ext/configuration.h b/ext/configuration.h index 57dd53cc7e..ccc4f549e6 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -156,7 +156,7 @@ enum ddtrace_sampling_rules_format { CONFIG(CUSTOM(INT), DD_TRACE_SAMPLING_RULES_FORMAT, "glob", .parser = dd_parse_sampling_rules_format) \ CONFIG(JSON, DD_SPAN_SAMPLING_RULES, "[]") \ CONFIG(STRING, DD_SPAN_SAMPLING_RULES_FILE, "", .ini_change = ddtrace_alter_sampling_rules_file_config) \ - CONFIG(SET_LOWERCASE, DD_TRACE_HEADER_TAGS, "", .ini_change = ddtrace_alter_DD_TRACE_HEADER_TAGS) \ + CONFIG(MAP_LOWERCASE, DD_TRACE_HEADER_TAGS, "", .ini_change = ddtrace_alter_DD_TRACE_HEADER_TAGS) \ CONFIG(INT, DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH, "512") \ CONFIG(MAP, DD_TRACE_PEER_SERVICE_MAPPING, "") \ CONFIG(BOOL, DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED, "false") \ @@ -274,6 +274,7 @@ typedef enum { DD_CONFIGURATION } ddtrace_config_id; } #define SET MAP #define SET_LOWERCASE MAP +#define MAP_LOWERCASE MAP #define JSON MAP #define MAP(id) \ static inline zend_array *get_##id(void) { return Z_ARR_P(zai_config_get_value(DDTRACE_CONFIG_##id)); } \ @@ -292,6 +293,7 @@ static inline bool get_global_DD_TRACE_SIDECAR_TRACE_SENDER(void) { return true; #undef STRING #undef MAP +#undef MAP_LOWERCASE #undef SET #undef SET_LOWERCASE #undef JSON diff --git a/ext/serializer.c b/ext/serializer.c index 173a87dd3b..705c944559 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -324,14 +324,20 @@ static zend_result dd_add_meta_array(void *context, ddtrace_string key, ddtrace_ static void dd_add_header_to_meta(zend_array *meta, const char *type, zend_string *lowerheader, zend_string *headerval) { - if (zend_hash_exists(get_DD_TRACE_HEADER_TAGS(), lowerheader)) { - for (char *ptr = ZSTR_VAL(lowerheader); *ptr; ++ptr) { - if ((*ptr < 'a' || *ptr > 'z') && *ptr != '-' && (*ptr < '0' || *ptr > '9')) { - *ptr = '_'; + zval *header_config = zend_hash_find(get_DD_TRACE_HEADER_TAGS(), lowerheader); + if (header_config != NULL && Z_TYPE_P(header_config) == IS_STRING) { + zend_string *header_config_str = Z_STR_P(header_config); + zend_string *headertag; + if (zend_string_equals(header_config_str, lowerheader)) { + for (char *ptr = ZSTR_VAL(header_config_str); *ptr; ++ptr) { + if ((*ptr < 'a' || *ptr > 'z') && *ptr != '-' && (*ptr < '0' || *ptr > '9')) { + *ptr = '_'; + } } + headertag = zend_strpprintf(0, "http.%s.headers.%s", type, ZSTR_VAL(header_config_str)); + } else { + headertag = zend_string_copy(header_config_str); } - - zend_string *headertag = zend_strpprintf(0, "http.%s.headers.%s", type, ZSTR_VAL(lowerheader)); zval headerzv; ZVAL_STR_COPY(&headerzv, headerval); zend_hash_update(meta, headertag, &headerzv); diff --git a/tests/Unit/ConfigurationTest.php b/tests/Unit/ConfigurationTest.php index 6fb10e4568..b810e5cb81 100644 --- a/tests/Unit/ConfigurationTest.php +++ b/tests/Unit/ConfigurationTest.php @@ -260,7 +260,7 @@ public function testGlobalTags() public function testGlobalTagsWrongValueJustResultsInNoTags() { $this->putEnvAndReloadConfig(['DD_TAGS=wrong_key_value']); - $this->assertEquals([], \dd_trace_env_config("DD_TAGS")); + $this->assertEquals(['wrong_key_value' => 'wrong_key_value'], \dd_trace_env_config("DD_TAGS")); } public function testHttpHeadersDefaultsToEmpty() @@ -279,10 +279,11 @@ public function testHttpHeadersCanSetOne() public function testHttpHeadersCanSetMultiple() { $this->putEnvAndReloadConfig([ - 'DD_TRACE_HEADER_TAGS=A-Header ,Any-Name , cOn7aining-!spe_cial?:ch/ars ', + 'DD_TRACE_HEADER_TAGS=A-Header ,Any-Name , cOn7aining-!spe_cial?:ch/ars , Some-Heather:with-colon-Key', ]); // Same behavior as python tracer: // https://github.com/DataDog/dd-trace-py/blob/f1298cb8100f146059f978b58c88641bd7424af8/ddtrace/http/headers.py - $this->assertSame(['a-header', 'any-name', 'con7aining-!spe_cial?:ch/ars'], array_keys(\dd_trace_env_config("DD_TRACE_HEADER_TAGS"))); + $this->assertSame(['a-header', 'any-name', 'con7aining-!spe_cial?', 'some-heather'], array_keys(\dd_trace_env_config("DD_TRACE_HEADER_TAGS"))); + $this->assertEquals(['a-header' => 'a-header', 'any-name' => 'any-name', 'con7aining-!spe_cial?' => 'ch/ars', 'some-heather' => 'with-colon-Key'], \dd_trace_env_config("DD_TRACE_HEADER_TAGS")); } } diff --git a/tests/ext/dd_trace_serialize_header_to_meta.phpt b/tests/ext/dd_trace_serialize_header_to_meta.phpt index 9146210e58..f9e92f26f4 100644 --- a/tests/ext/dd_trace_serialize_header_to_meta.phpt +++ b/tests/ext/dd_trace_serialize_header_to_meta.phpt @@ -4,7 +4,7 @@ Headers values are mapped to expected tag key HTTP_CONTENT_TYPE=text/plain HTTP_CUSTOM_HEADER=custom-header-value DD_TRACE_GENERATE_ROOT_SPAN=0 -DD_TRACE_HEADER_TAGS=Content-Type,Custom-Header:custom-header-key +DD_TRACE_HEADER_TAGS=Content-Type,Custom-Header:custom-HeaderKey --GET-- application_key=123 --FILE-- @@ -13,8 +13,8 @@ DDTrace\start_span(); DDTrace\close_span(); $spans = dd_trace_serialize_closed_spans(); var_dump($spans[0]['meta']['http.request.headers.content-type']); -var_dump($spans[0]['meta']['custom-header-key']); +var_dump($spans[0]['meta']['custom-HeaderKey']); ?> --EXPECT-- string(10) "text/plain" -string(13) "custom-header-value" +string(19) "custom-header-value" diff --git a/zend_abstract_interface/config/config_decode.c b/zend_abstract_interface/config/config_decode.c index 6daa3bb5f0..926567e1ca 100644 --- a/zend_abstract_interface/config/config_decode.c +++ b/zend_abstract_interface/config/config_decode.c @@ -105,7 +105,7 @@ static bool zai_config_decode_int(zai_str value, zval *decoded_value) { return true; } -static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persistent) { +static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persistent, bool lowercase) { zval tmp; ZVAL_ARR(&tmp, pemalloc(sizeof(HashTable), persistent)); zend_hash_init(Z_ARRVAL(tmp), 8, NULL, persistent ? ZVAL_INTERNAL_PTR_DTOR : ZVAL_PTR_DTOR, persistent); @@ -113,38 +113,70 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi char *data = (char *)value.ptr; if (data && *data) { // non-empty const char *key_start, *key_end, *value_start, *value_end; + do { - if (*data != ',' && *data != ' ' && *data != '\t' && *data != '\n') { - key_start = key_end = data; - while (*++data) { + while (*data == ',' || *data == ' ' || *data == '\t' || *data == '\n') { + data++; + } + + if (*data) { + key_start = data; + key_end = NULL; + bool has_colon = false; + + while (*data && *data != ',') { if (*data == ':') { - while (*++data && (*data == ' ' || *data == '\t' || *data == '\n')) - ; - - value_start = value_end = data; - if (!*data || *data == ',') { - --value_end; // empty string instead of single char - } else { - while (*++data && *data != ',') { - if (*data != ' ' && *data != '\t' && *data != '\n') { - value_end = data; - } + has_colon = true; + + data++; + while (*data == ' ' || *data == '\t' || *data == '\n') data++; + + value_start = data; + value_end = value_start; + + while (*data && *data != ',') { + if (*data != ' ' && *data != '\t' && *data != '\n') { + value_end = data; } + data++; } size_t key_len = key_end - key_start + 1; size_t value_len = value_end - value_start + 1; - zval val; - ZVAL_NEW_STR(&val, zend_string_init(value_start, value_len, persistent)); - zend_hash_str_update(Z_ARRVAL(tmp), key_start, key_len, &val); + + if (key_len > 0 && value_len > 0) { + zend_string *key = zend_string_init(key_start, key_len, persistent); + if (lowercase) { + zend_str_tolower(ZSTR_VAL(key), ZSTR_LEN(key)); + } + + zval val; + ZVAL_NEW_STR(&val, zend_string_init(value_start, value_len, persistent)); + zend_hash_update(Z_ARRVAL(tmp), key, &val); + zend_string_release(key); + } break; } + if (*data != ' ' && *data != '\t' && *data != '\n') { key_end = data; } + data++; + } + + // Handle case without a colon (standalone key) + if (!has_colon && key_end) { + size_t key_len = key_end - key_start + 1; + zend_string *key = zend_string_init(key_start, key_len, persistent); + if (lowercase) { + zend_str_tolower(ZSTR_VAL(key), ZSTR_LEN(key)); + } + + zval val; + ZVAL_NEW_STR(&val, zend_string_copy(key)); + zend_hash_update(Z_ARRVAL(tmp), key, &val); + zend_string_release(key); } - } else { - ++data; } } while (*data); @@ -232,7 +264,9 @@ bool zai_config_decode_value(zai_str value, zai_config_type type, zai_custom_par case ZAI_CONFIG_TYPE_INT: return zai_config_decode_int(value, decoded_value); case ZAI_CONFIG_TYPE_MAP: - return zai_config_decode_map(value, decoded_value, persistent); + return zai_config_decode_map(value, decoded_value, persistent, false); + case ZAI_CONFIG_TYPE_MAP_LOWERCASE: + return zai_config_decode_map(value, decoded_value, persistent, true); case ZAI_CONFIG_TYPE_SET: return zai_config_decode_set(value, decoded_value, persistent, false); case ZAI_CONFIG_TYPE_SET_LOWERCASE: diff --git a/zend_abstract_interface/config/config_decode.h b/zend_abstract_interface/config/config_decode.h index 754bb43a01..95a12d8db6 100644 --- a/zend_abstract_interface/config/config_decode.h +++ b/zend_abstract_interface/config/config_decode.h @@ -11,6 +11,7 @@ typedef enum { ZAI_CONFIG_TYPE_DOUBLE, ZAI_CONFIG_TYPE_INT, ZAI_CONFIG_TYPE_MAP, + ZAI_CONFIG_TYPE_MAP_LOWERCASE, ZAI_CONFIG_TYPE_SET, ZAI_CONFIG_TYPE_SET_LOWERCASE, ZAI_CONFIG_TYPE_JSON, From 7b3c423294d283340e9f781ca7ce7138be8e9f1c Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Tue, 5 Nov 2024 13:44:19 -0500 Subject: [PATCH 3/8] Using lowercase condition for new map and empty value for keyless header --- ext/serializer.c | 6 +++--- tests/Unit/ConfigurationTest.php | 4 ++-- zend_abstract_interface/config/config_decode.c | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ext/serializer.c b/ext/serializer.c index 705c944559..328e140ed8 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -328,13 +328,13 @@ static void dd_add_header_to_meta(zend_array *meta, const char *type, zend_strin if (header_config != NULL && Z_TYPE_P(header_config) == IS_STRING) { zend_string *header_config_str = Z_STR_P(header_config); zend_string *headertag; - if (zend_string_equals(header_config_str, lowerheader)) { - for (char *ptr = ZSTR_VAL(header_config_str); *ptr; ++ptr) { + if (ZSTR_LEN(header_config_str) == 0) { + for (char *ptr = ZSTR_VAL(lowerheader); *ptr; ++ptr) { if ((*ptr < 'a' || *ptr > 'z') && *ptr != '-' && (*ptr < '0' || *ptr > '9')) { *ptr = '_'; } } - headertag = zend_strpprintf(0, "http.%s.headers.%s", type, ZSTR_VAL(header_config_str)); + headertag = zend_strpprintf(0, "http.%s.headers.%s", type, ZSTR_VAL(lowerheader)); } else { headertag = zend_string_copy(header_config_str); } diff --git a/tests/Unit/ConfigurationTest.php b/tests/Unit/ConfigurationTest.php index b810e5cb81..eb0100c353 100644 --- a/tests/Unit/ConfigurationTest.php +++ b/tests/Unit/ConfigurationTest.php @@ -260,7 +260,7 @@ public function testGlobalTags() public function testGlobalTagsWrongValueJustResultsInNoTags() { $this->putEnvAndReloadConfig(['DD_TAGS=wrong_key_value']); - $this->assertEquals(['wrong_key_value' => 'wrong_key_value'], \dd_trace_env_config("DD_TAGS")); + $this->assertEquals([], \dd_trace_env_config("DD_TAGS")); } public function testHttpHeadersDefaultsToEmpty() @@ -284,6 +284,6 @@ public function testHttpHeadersCanSetMultiple() // Same behavior as python tracer: // https://github.com/DataDog/dd-trace-py/blob/f1298cb8100f146059f978b58c88641bd7424af8/ddtrace/http/headers.py $this->assertSame(['a-header', 'any-name', 'con7aining-!spe_cial?', 'some-heather'], array_keys(\dd_trace_env_config("DD_TRACE_HEADER_TAGS"))); - $this->assertEquals(['a-header' => 'a-header', 'any-name' => 'any-name', 'con7aining-!spe_cial?' => 'ch/ars', 'some-heather' => 'with-colon-Key'], \dd_trace_env_config("DD_TRACE_HEADER_TAGS")); + $this->assertEquals(['a-header' => '', 'any-name' => '', 'con7aining-!spe_cial?' => 'ch/ars', 'some-heather' => 'with-colon-Key'], \dd_trace_env_config("DD_TRACE_HEADER_TAGS")); } } diff --git a/zend_abstract_interface/config/config_decode.c b/zend_abstract_interface/config/config_decode.c index 926567e1ca..6a56a96adc 100644 --- a/zend_abstract_interface/config/config_decode.c +++ b/zend_abstract_interface/config/config_decode.c @@ -164,8 +164,8 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi data++; } - // Handle case without a colon (standalone key) - if (!has_colon && key_end) { + // Handle case without a colon (standalone key) only when lowercase is true which is current use case + if (lowercase && !has_colon && key_end) { size_t key_len = key_end - key_start + 1; zend_string *key = zend_string_init(key_start, key_len, persistent); if (lowercase) { @@ -173,7 +173,7 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi } zval val; - ZVAL_NEW_STR(&val, zend_string_copy(key)); + ZVAL_EMPTY_STRING(&val); zend_hash_update(Z_ARRVAL(tmp), key, &val); zend_string_release(key); } From e1d305bd50041f9ce1cbc0bed0595988add35321 Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Tue, 5 Nov 2024 17:16:13 -0500 Subject: [PATCH 4/8] Adding easy nits --- ext/configuration.h | 6 +++--- zend_abstract_interface/config/config_decode.c | 10 +++++----- zend_abstract_interface/config/config_decode.h | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ext/configuration.h b/ext/configuration.h index ccc4f549e6..e94d885b7a 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -156,7 +156,7 @@ enum ddtrace_sampling_rules_format { CONFIG(CUSTOM(INT), DD_TRACE_SAMPLING_RULES_FORMAT, "glob", .parser = dd_parse_sampling_rules_format) \ CONFIG(JSON, DD_SPAN_SAMPLING_RULES, "[]") \ CONFIG(STRING, DD_SPAN_SAMPLING_RULES_FILE, "", .ini_change = ddtrace_alter_sampling_rules_file_config) \ - CONFIG(MAP_LOWERCASE, DD_TRACE_HEADER_TAGS, "", .ini_change = ddtrace_alter_DD_TRACE_HEADER_TAGS) \ + CONFIG(SET_OR_MAP_LOWERCASE, DD_TRACE_HEADER_TAGS, "", .ini_change = ddtrace_alter_DD_TRACE_HEADER_TAGS) \ CONFIG(INT, DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH, "512") \ CONFIG(MAP, DD_TRACE_PEER_SERVICE_MAPPING, "") \ CONFIG(BOOL, DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED, "false") \ @@ -274,7 +274,7 @@ typedef enum { DD_CONFIGURATION } ddtrace_config_id; } #define SET MAP #define SET_LOWERCASE MAP -#define MAP_LOWERCASE MAP +#define SET_OR_MAP_LOWERCASE MAP #define JSON MAP #define MAP(id) \ static inline zend_array *get_##id(void) { return Z_ARR_P(zai_config_get_value(DDTRACE_CONFIG_##id)); } \ @@ -293,9 +293,9 @@ static inline bool get_global_DD_TRACE_SIDECAR_TRACE_SENDER(void) { return true; #undef STRING #undef MAP -#undef MAP_LOWERCASE #undef SET #undef SET_LOWERCASE +#undef SET_OR_MAP_LOWERCASE #undef JSON #undef BOOL #undef INT diff --git a/zend_abstract_interface/config/config_decode.c b/zend_abstract_interface/config/config_decode.c index 6a56a96adc..b94156d3e0 100644 --- a/zend_abstract_interface/config/config_decode.c +++ b/zend_abstract_interface/config/config_decode.c @@ -105,7 +105,7 @@ static bool zai_config_decode_int(zai_str value, zval *decoded_value) { return true; } -static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persistent, bool lowercase) { +static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persistent, bool lowercase, bool map_keyless) { zval tmp; ZVAL_ARR(&tmp, pemalloc(sizeof(HashTable), persistent)); zend_hash_init(Z_ARRVAL(tmp), 8, NULL, persistent ? ZVAL_INTERNAL_PTR_DTOR : ZVAL_PTR_DTOR, persistent); @@ -165,7 +165,7 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi } // Handle case without a colon (standalone key) only when lowercase is true which is current use case - if (lowercase && !has_colon && key_end) { + if (map_keyless && !has_colon && key_end) { size_t key_len = key_end - key_start + 1; zend_string *key = zend_string_init(key_start, key_len, persistent); if (lowercase) { @@ -264,9 +264,9 @@ bool zai_config_decode_value(zai_str value, zai_config_type type, zai_custom_par case ZAI_CONFIG_TYPE_INT: return zai_config_decode_int(value, decoded_value); case ZAI_CONFIG_TYPE_MAP: - return zai_config_decode_map(value, decoded_value, persistent, false); - case ZAI_CONFIG_TYPE_MAP_LOWERCASE: - return zai_config_decode_map(value, decoded_value, persistent, true); + return zai_config_decode_map(value, decoded_value, persistent, false, false); + case ZAI_CONFIG_TYPE_SET_OR_MAP_LOWERCASE: + return zai_config_decode_map(value, decoded_value, persistent, true, true); case ZAI_CONFIG_TYPE_SET: return zai_config_decode_set(value, decoded_value, persistent, false); case ZAI_CONFIG_TYPE_SET_LOWERCASE: diff --git a/zend_abstract_interface/config/config_decode.h b/zend_abstract_interface/config/config_decode.h index 95a12d8db6..c811b37df2 100644 --- a/zend_abstract_interface/config/config_decode.h +++ b/zend_abstract_interface/config/config_decode.h @@ -11,9 +11,9 @@ typedef enum { ZAI_CONFIG_TYPE_DOUBLE, ZAI_CONFIG_TYPE_INT, ZAI_CONFIG_TYPE_MAP, - ZAI_CONFIG_TYPE_MAP_LOWERCASE, ZAI_CONFIG_TYPE_SET, ZAI_CONFIG_TYPE_SET_LOWERCASE, + ZAI_CONFIG_TYPE_SET_OR_MAP_LOWERCASE, ZAI_CONFIG_TYPE_JSON, ZAI_CONFIG_TYPE_STRING, ZAI_CONFIG_TYPE_CUSTOM, From ea54b0ed35e6d57da4057583551a871261b6d34a Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Tue, 5 Nov 2024 20:16:20 -0500 Subject: [PATCH 5/8] Updated to take into account empty string after colon and safer use of ZVAL methods --- tests/Unit/ConfigurationTest.php | 8 ++- .../config/config_decode.c | 51 +++++++++++++------ 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/tests/Unit/ConfigurationTest.php b/tests/Unit/ConfigurationTest.php index eb0100c353..c96ceaacb3 100644 --- a/tests/Unit/ConfigurationTest.php +++ b/tests/Unit/ConfigurationTest.php @@ -279,11 +279,9 @@ public function testHttpHeadersCanSetOne() public function testHttpHeadersCanSetMultiple() { $this->putEnvAndReloadConfig([ - 'DD_TRACE_HEADER_TAGS=A-Header ,Any-Name , cOn7aining-!spe_cial?:ch/ars , Some-Heather:with-colon-Key', + 'DD_TRACE_HEADER_TAGS=A-Header ,Any-Name , cOn7aining-!spe_cial?:ch/ars , valueless:, Some-Header:with-colon-Key', ]); - // Same behavior as python tracer: - // https://github.com/DataDog/dd-trace-py/blob/f1298cb8100f146059f978b58c88641bd7424af8/ddtrace/http/headers.py - $this->assertSame(['a-header', 'any-name', 'con7aining-!spe_cial?', 'some-heather'], array_keys(\dd_trace_env_config("DD_TRACE_HEADER_TAGS"))); - $this->assertEquals(['a-header' => '', 'any-name' => '', 'con7aining-!spe_cial?' => 'ch/ars', 'some-heather' => 'with-colon-Key'], \dd_trace_env_config("DD_TRACE_HEADER_TAGS")); + $this->assertSame(['a-header', 'any-name', 'con7aining-!spe_cial?', 'valueless', 'some-header'], array_keys(\dd_trace_env_config("DD_TRACE_HEADER_TAGS"))); + $this->assertEquals(['a-header' => '', 'any-name' => '', 'con7aining-!spe_cial?' => 'ch/ars', 'valueless' => '', 'some-header' => 'with-colon-Key'], \dd_trace_env_config("DD_TRACE_HEADER_TAGS")); } } diff --git a/zend_abstract_interface/config/config_decode.c b/zend_abstract_interface/config/config_decode.c index b94156d3e0..aa2080bfec 100644 --- a/zend_abstract_interface/config/config_decode.c +++ b/zend_abstract_interface/config/config_decode.c @@ -113,7 +113,7 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi char *data = (char *)value.ptr; if (data && *data) { // non-empty const char *key_start, *key_end, *value_start, *value_end; - + do { while (*data == ',' || *data == ' ' || *data == '\t' || *data == '\n') { data++; @@ -127,45 +127,59 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi while (*data && *data != ',') { if (*data == ':') { has_colon = true; - data++; + while (*data == ' ' || *data == '\t' || *data == '\n') data++; - value_start = data; - value_end = value_start; - - while (*data && *data != ',') { - if (*data != ' ' && *data != '\t' && *data != '\n') { - value_end = data; + if (*data == ',' || *data == '\0') { + value_start = value_end = NULL; + } else { + value_start = data; + value_end = value_start; + + while (*data && *data != ',') { + if (*data != ' ' && *data != '\t' && *data != '\n') { + value_end = data; + } + data++; } - data++; } - size_t key_len = key_end - key_start + 1; - size_t value_len = value_end - value_start + 1; + if (key_end && key_start) { + size_t key_len = key_end - key_start + 1; + size_t value_len = (value_end && value_start) ? (value_end - value_start + 1) : 0; - if (key_len > 0 && value_len > 0) { zend_string *key = zend_string_init(key_start, key_len, persistent); if (lowercase) { zend_str_tolower(ZSTR_VAL(key), ZSTR_LEN(key)); } zval val; - ZVAL_NEW_STR(&val, zend_string_init(value_start, value_len, persistent)); + if (value_len > 0) { + ZVAL_NEW_STR(&val, zend_string_init(value_start, value_len, persistent)); + } else { + if (persistent) { + ZVAL_EMPTY_PSTRING(&val); + } else { + ZVAL_EMPTY_STRING(&val); + } + } zend_hash_update(Z_ARRVAL(tmp), key, &val); zend_string_release(key); } + break; } + // Set key_end to the last valid non-whitespace character of the key if (*data != ' ' && *data != '\t' && *data != '\n') { key_end = data; } data++; } - // Handle case without a colon (standalone key) only when lowercase is true which is current use case - if (map_keyless && !has_colon && key_end) { + // Handle standalone keys (without a colon) if map_keyless is enabled + if (map_keyless && !has_colon && key_end && key_start) { size_t key_len = key_end - key_start + 1; zend_string *key = zend_string_init(key_start, key_len, persistent); if (lowercase) { @@ -173,13 +187,18 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi } zval val; - ZVAL_EMPTY_STRING(&val); + if (persistent) { + ZVAL_EMPTY_PSTRING(&val); + } else { + ZVAL_EMPTY_STRING(&val); + } zend_hash_update(Z_ARRVAL(tmp), key, &val); zend_string_release(key); } } } while (*data); + // Check if the array has any elements; if not, cleanup if (zend_hash_num_elements(Z_ARRVAL(tmp)) == 0) { zend_hash_destroy(Z_ARRVAL(tmp)); pefree(Z_ARRVAL(tmp), persistent); From 0de558f64ef4a156ba377c3ea35f7a69017bad86 Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Wed, 6 Nov 2024 12:25:17 -0500 Subject: [PATCH 6/8] Applying nits changes --- zend_abstract_interface/config/config_decode.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/zend_abstract_interface/config/config_decode.c b/zend_abstract_interface/config/config_decode.c index aa2080bfec..c95a971d6b 100644 --- a/zend_abstract_interface/config/config_decode.c +++ b/zend_abstract_interface/config/config_decode.c @@ -131,23 +131,20 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi while (*data == ' ' || *data == '\t' || *data == '\n') data++; - if (*data == ',' || *data == '\0') { - value_start = value_end = NULL; + if (*data == ',' || !*data) { + value_end = NULL; } else { - value_start = data; - value_end = value_start; - - while (*data && *data != ',') { + value_start = value_end = data; + do { if (*data != ' ' && *data != '\t' && *data != '\n') { value_end = data; } data++; - } + } while (*data && *data != ','); } if (key_end && key_start) { size_t key_len = key_end - key_start + 1; - size_t value_len = (value_end && value_start) ? (value_end - value_start + 1) : 0; zend_string *key = zend_string_init(key_start, key_len, persistent); if (lowercase) { @@ -155,7 +152,8 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi } zval val; - if (value_len > 0) { + if (value_end) { + size_t value_len = value_end ? (value_end - value_start + 1) : 0; ZVAL_NEW_STR(&val, zend_string_init(value_start, value_len, persistent)); } else { if (persistent) { @@ -179,7 +177,7 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi } // Handle standalone keys (without a colon) if map_keyless is enabled - if (map_keyless && !has_colon && key_end && key_start) { + if (map_keyless && !has_colon && key_end) { size_t key_len = key_end - key_start + 1; zend_string *key = zend_string_init(key_start, key_len, persistent); if (lowercase) { From c26eda07250b1b29c8e4c3dcf1a5b85b8c0ae6cd Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Wed, 6 Nov 2024 12:37:01 -0500 Subject: [PATCH 7/8] Adding spacing tests based on system-test examples --- tests/ext/dd_trace_serialize_header_to_meta.phpt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/ext/dd_trace_serialize_header_to_meta.phpt b/tests/ext/dd_trace_serialize_header_to_meta.phpt index f9e92f26f4..f8e548765a 100644 --- a/tests/ext/dd_trace_serialize_header_to_meta.phpt +++ b/tests/ext/dd_trace_serialize_header_to_meta.phpt @@ -3,8 +3,10 @@ Headers values are mapped to expected tag key --ENV-- HTTP_CONTENT_TYPE=text/plain HTTP_CUSTOM_HEADER=custom-header-value +HTTP_HEADER1=val +HTTP_HEADER2=v a l DD_TRACE_GENERATE_ROOT_SPAN=0 -DD_TRACE_HEADER_TAGS=Content-Type,Custom-Header:custom-HeaderKey +DD_TRACE_HEADER_TAGS=Content-Type,Custom-Header:custom-HeaderKey,header1: t a g ,header2:tag --GET-- application_key=123 --FILE-- @@ -14,7 +16,11 @@ DDTrace\close_span(); $spans = dd_trace_serialize_closed_spans(); var_dump($spans[0]['meta']['http.request.headers.content-type']); var_dump($spans[0]['meta']['custom-HeaderKey']); +var_dump($spans[0]['meta']['t a g']); +var_dump($spans[0]['meta']['tag']); ?> --EXPECT-- string(10) "text/plain" string(19) "custom-header-value" +string(3) "val" +string(5) "v a l" From 2f104b2484299e5d02cf979a9a3c3ae39bdf2ee6 Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Wed, 6 Nov 2024 13:55:00 -0500 Subject: [PATCH 8/8] Removing redundant check --- zend_abstract_interface/config/config_decode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zend_abstract_interface/config/config_decode.c b/zend_abstract_interface/config/config_decode.c index c95a971d6b..552360f657 100644 --- a/zend_abstract_interface/config/config_decode.c +++ b/zend_abstract_interface/config/config_decode.c @@ -153,7 +153,7 @@ static bool zai_config_decode_map(zai_str value, zval *decoded_value, bool persi zval val; if (value_end) { - size_t value_len = value_end ? (value_end - value_start + 1) : 0; + size_t value_len = value_end - value_start + 1; ZVAL_NEW_STR(&val, zend_string_init(value_start, value_len, persistent)); } else { if (persistent) {