From 229251b6864373633047a5c9665320a259968268 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Wed, 13 May 2026 13:40:38 -0400 Subject: [PATCH 1/2] fix(profiling): drop malformed info / internal_metadata JSON instead of failing the upload The `ddog_prof_Exporter_send_blocking` and async `send` entry points used to call `parse_json(...)?` for both `optional_internal_metadata_json` and `optional_info_json`, which propagated a parse error all the way back to the caller. In practice that meant a malformed supplementary JSON blob (e.g. a stray `Infinity` from Python's permissive `json.dumps`, or an integer that exceeds the range serde_json can represent) caused the entire pprof profile to be dropped on the floor. Replace `parse_json` with `parse_json_lossy`, which logs a warning and returns `None` on either invalid UTF-8 or a serde_json parse failure. The profile, which is the primary artifact, still uploads with whatever supplementary fields parsed successfully. This matches what callers already do on their side: dd-trace-py wraps the `json.dumps` of its `info.profiler.settings` payload in a try/except and swallows failures, on the assumption that a bad metadata blob is not worth losing a profile over. The native side now has the same semantics. * `parse_json_lossy` is private to the module and replaces the previous `parse_json`. No other crate referenced the old function. * Logging uses `eprintln!` to match the style in `libdd-profiling/src/exporter/exporter_manager.rs`. * Three new unit tests cover the None / valid / invalid paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- libdd-profiling-ffi/src/exporter.rs | 71 +++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 19 deletions(-) diff --git a/libdd-profiling-ffi/src/exporter.rs b/libdd-profiling-ffi/src/exporter.rs index 51a984c782..5923558e99 100644 --- a/libdd-profiling-ffi/src/exporter.rs +++ b/libdd-profiling-ffi/src/exporter.rs @@ -201,23 +201,31 @@ unsafe fn into_vec_files<'a>(slice: Slice<'a, File>) -> Vec> .collect() } -unsafe fn parse_json( +/// Best-effort parse of an optional caller-supplied JSON blob. On invalid +/// UTF-8 or a `serde_json` parse error, log a warning and return `None` +/// instead of propagating the error. The `info` and `internal_metadata` +/// channels are supplementary signals attached to the profile upload event; +/// a malformed payload there should not cause us to drop the pprof itself, +/// which is the primary artifact. +unsafe fn parse_json_lossy( string_id: &str, json_string: Option<&CharSlice>, -) -> anyhow::Result> { - match json_string { - None => Ok(None), - Some(json_string) => { - let json = json_string.try_to_utf8()?; - match serde_json::from_str(json) { - Ok(parsed) => Ok(Some(parsed)), - Err(error) => Err(anyhow::anyhow!( - "Failed to parse contents of {} json string (`{}`): {}.", - string_id, - json, - error - )), - } +) -> Option { + let cs = json_string?; + let json = match cs.try_to_utf8() { + Ok(s) => s, + Err(err) => { + eprintln!("warning: profile {string_id} JSON is not valid UTF-8, dropping it: {err}"); + return None; + } + }; + match serde_json::from_str(json) { + Ok(parsed) => Some(parsed), + Err(err) => { + eprintln!( + "warning: failed to parse profile {string_id} JSON (`{json}`), dropping it: {err}" + ); + None } } } @@ -289,8 +297,9 @@ pub unsafe extern "C" fn ddog_prof_Exporter_send_blocking( let process_tags_str = optional_process_tags .map(|cs| cs.try_to_utf8()) .transpose()?; - let internal_metadata = parse_json("internal_metadata", optional_internal_metadata_json)?; - let info = parse_json("info", optional_info_json)?; + let internal_metadata = + parse_json_lossy("internal_metadata", optional_internal_metadata_json); + let info = parse_json_lossy("info", optional_info_json); let cancel = cancel.to_inner_mut().ok(); let status = exporter.send_blocking( @@ -443,8 +452,9 @@ pub unsafe extern "C" fn ddog_prof_ExporterManager_queue( let process_tags_str = optional_process_tags .map(|cs| cs.try_to_utf8()) .transpose()?; - let internal_metadata = parse_json("internal_metadata", optional_internal_metadata_json)?; - let info = parse_json("info", optional_info_json)?; + let internal_metadata = + parse_json_lossy("internal_metadata", optional_internal_metadata_json); + let info = parse_json_lossy("info", optional_info_json); manager.queue( profile, @@ -766,4 +776,27 @@ mod tests { } } } + + #[test] + fn test_parse_json_lossy_none() { + let parsed = unsafe { parse_json_lossy("info", None) }; + assert!(parsed.is_none()); + } + + #[test] + fn test_parse_json_lossy_valid() { + let s = CharSlice::from(r#"{"runtime": {"engine": "test"}}"#); + let parsed = unsafe { parse_json_lossy("info", Some(&s)) }.expect("valid JSON"); + assert_eq!(parsed["runtime"]["engine"], "test"); + } + + #[test] + fn test_parse_json_lossy_drops_invalid() { + // Malformed JSON: trailing comma + missing closing brace. Must not + // propagate an error; we should silently drop the payload so the + // pprof itself can still upload. + let s = CharSlice::from(r#"{"runtime": {"engine": "test",}"#); + let parsed = unsafe { parse_json_lossy("info", Some(&s)) }; + assert!(parsed.is_none()); + } } From f5b16801c7dacb5903c0ef01c272b6b4d5e2c2b0 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Wed, 13 May 2026 14:21:28 -0400 Subject: [PATCH 2/2] fix(profiling): redact JSON payload from parse warning, cover UTF-8 branch Addresses PR review feedback: - Avoid logging the entire caller-provided JSON blob on parse failure; it may contain runtime/profiler configuration. Log only the field name and the serde_json error. - Add a test for the invalid-UTF-8 branch of parse_json_lossy so the early-return path is covered. Co-Authored-By: Claude Opus 4.7 (1M context) --- libdd-profiling-ffi/src/exporter.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libdd-profiling-ffi/src/exporter.rs b/libdd-profiling-ffi/src/exporter.rs index 5923558e99..e10a63dff8 100644 --- a/libdd-profiling-ffi/src/exporter.rs +++ b/libdd-profiling-ffi/src/exporter.rs @@ -222,9 +222,7 @@ unsafe fn parse_json_lossy( match serde_json::from_str(json) { Ok(parsed) => Some(parsed), Err(err) => { - eprintln!( - "warning: failed to parse profile {string_id} JSON (`{json}`), dropping it: {err}" - ); + eprintln!("warning: failed to parse profile {string_id} JSON, dropping it: {err}"); None } } @@ -799,4 +797,13 @@ mod tests { let parsed = unsafe { parse_json_lossy("info", Some(&s)) }; assert!(parsed.is_none()); } + + #[test] + fn test_parse_json_lossy_drops_invalid_utf8() { + // 0xFF is never valid in UTF-8. Must be dropped, not propagated. + let raw: &[u8] = &[b'{', 0xFF, b'}']; + let s = CharSlice::from_bytes(raw); + let parsed = unsafe { parse_json_lossy("info", Some(&s)) }; + assert!(parsed.is_none()); + } }