Ensure the msgpack contents include only valid utf8 (#2698)

bwoebi · pierotibou · web-flow · commit 8d07c879e604 · 2024-06-08T22:20:36.000+02:00
* Ensure the msgpack contents include only valid utf8

Signed-off-by: Bob Weinand &lt;bob.weinand@datadoghq.com&gt;

* Update ext/serializer.c

Co-authored-by: Pierre Bonet &lt;pierotibou@gmail.com&gt;

* Avoid minor perf impact on msgpack serialization with sidecar sender disabled

Signed-off-by: Bob Weinand &lt;bob.weinand@datadoghq.com&gt;

---------

Signed-off-by: Bob Weinand &lt;bob.weinand@datadoghq.com&gt;
Co-authored-by: Pierre Bonet &lt;pierotibou@gmail.com&gt;
diff --git a/components-rs/ddtrace.h b/components-rs/ddtrace.h
@@ -139,6 +139,10 @@ ddog_CharSlice ddtrace_get_container_id(void);
 
 void ddtrace_set_container_cgroup_path(ddog_CharSlice path);
 
+char *ddtrace_strip_invalid_utf8(const char *input, uintptr_t *len);
+
+void ddtrace_drop_rust_string(char *input, uintptr_t len);
+
 bool ddog_shall_log(enum ddog_Log category);
 
 void ddog_set_error_log_level(bool once);
diff --git a/components-rs/lib.rs b/components-rs/lib.rs
@@ -5,6 +5,9 @@ pub mod log;
 pub mod sidecar;
 pub mod telemetry;
 
+use std::borrow::Cow;
+use std::ffi::c_char;
+use std::ptr::null_mut;
 use ddcommon::entity_id::{get_container_id, set_cgroup_file};
 use ddcommon_ffi::CharSlice;
 use uuid::Uuid;
@@ -42,3 +45,21 @@ pub extern "C" fn ddtrace_get_container_id() -> CharSlice<'static> {
 pub unsafe extern "C" fn ddtrace_set_container_cgroup_path(path: CharSlice) {
     set_cgroup_file(String::from(path.try_to_utf8().unwrap()))
 }
+
+#[no_mangle]
+pub unsafe extern "C" fn ddtrace_strip_invalid_utf8(input: *const c_char, len: *mut usize) -> *mut c_char {
+    match CharSlice::from_raw_parts(input, *len).to_utf8_lossy() {
+        Cow::Borrowed(_) => null_mut(),
+        Cow::Owned(s) => {
+            *len = s.len();
+            let ret = s.as_ptr() as *mut c_char;
+            std::mem::forget(s);
+            ret
+        }
+    }
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn ddtrace_drop_rust_string(input: *mut c_char, len: usize) {
+    _ = String::from_raw_parts(input as *mut u8, len, len);
+}
diff --git a/ext/serializer.c b/ext/serializer.c
@@ -46,6 +46,19 @@ extern void (*profiling_notify_trace_finished)(uint64_t local_root_span_id,
                                                zai_str span_type,
                                                zai_str resource);
 
+static void mpack_write_utf8_lossy_cstr(mpack_writer_t *writer, const char *str, size_t len) {
+    if (get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) {
+        char *strippedStr = ddtrace_strip_invalid_utf8(str, &len);
+        if (strippedStr) {
+            mpack_write_str(writer, strippedStr, len);
+            ddtrace_drop_rust_string(strippedStr, len);
+            return;
+        }
+    }
+
+    mpack_write_str(writer, str, len);
+}
+
 #define MAX_ID_BUFSIZ 40  // 3.4e^38 = 39 chars + 1 terminator
 #define KEY_TRACE_ID "trace_id"
 #define KEY_SPAN_ID "span_id"
@@ -78,13 +91,15 @@ static int write_hash_table(mpack_writer_t *writer, HashTable *ht, int level) {
         bool zval_string_as_uint64 = false;
         if (is_assoc == 1) {
             char num_str_buf[MAX_ID_BUFSIZ], *key;
+            size_t len;
             if (string_key) {
                 key = ZSTR_VAL(string_key);
+                len = ZSTR_LEN(string_key);
             } else {
                 key = num_str_buf;
-                sprintf(num_str_buf, ZEND_LONG_FMT, num_key);
+                len = sprintf(num_str_buf, ZEND_LONG_FMT, num_key);
             }
-            mpack_write_cstr(writer, key);
+            mpack_write_utf8_lossy_cstr(writer, key, len);
             // If the key is trace_id, span_id or parent_id then strings have to be converted to uint64 when packed.
             if (level <= 3 &&
                 (0 == strcmp(KEY_TRACE_ID, key) || 0 == strcmp(KEY_SPAN_ID, key) || 0 == strcmp(KEY_PARENT_ID, key))) {
@@ -134,7 +149,7 @@ static int msgpack_write_zval(mpack_writer_t *writer, zval *trace, int level) {
             mpack_write_bool(writer, Z_TYPE_P(trace) == IS_TRUE);
             break;
         case IS_STRING:
-            mpack_write_cstr(writer, Z_STRVAL_P(trace));
+            mpack_write_utf8_lossy_cstr(writer, Z_STRVAL_P(trace), Z_STRLEN_P(trace));
             break;
         default:
             LOG(WARN, "Serialize values must be of type array, string, int, float, bool or null");
