Skip to content

Commit 02d95c0

Browse files
authored
fix(crashtracking): handle new lines in client submitted exception message (#1836)
# What does this PR do? Escapes new lines from API consumer submitted unhandled exception message, then unescapes it when writing the report. This is necessary, because API consumers can submit arbitrary exception messages, and if they contain new lines, the receiver state machine parsing breaks as it depends on chunks of data being sent as lines. # Motivation This [crash log](https://app.datadoghq.com/logs?query=service%3Ainstrumentation-telemetry-data%20%28%40tags.severity%3Acrash%20OR%20severity%3Acrash%20OR%20signum%3A%2A%20OR%20%40error.is_crash%3Atrue%29%20-%40metadata.tags%3A%22crash_datadog%3Atrue%22%20-%22Process%20was%20terminated%20due%20to%20an%20unhandled%20exception%22%20source%3Aruby&agg_m=count&agg_m_source=base&agg_q=%40metadata.family&agg_t=count&cf_lbyyhhwhyjj5l3rs65cb3w=aiugo4c9e5i04jdlqn4i&clustering_pattern_field_path=message&cols=source%2C%40tracer_version%2C%40language_version%2C%40org&event=AwAAAZ0xDw1VSu-b3AAAABhBWjB4RHcxVkFBQkxSUWRlaE1Rb1B3QUEAAAAkZDE5ZDMxMGYtNGI0OC00NDQ5LWI5NmMtY2QwZjJkNjE0NDExAAAfZw&flat_group_bys=true&fromUser=true&messageDisplay=inline&refresh_mode=sliding&sort_m=count&sort_t=count&storage=live&stream_sort=desc&top_n=10&top_o=top&view=spans&viz=stream&x_missing=true&from_ts=1772048747006&to_ts=1774640747006&live=true) has no message because the exception message being sent has [new lines](https://github.com/rails/rails/blob/8ec9ba082fd08f7db0c601056d86356b0b5e4d25/activerecord/lib/active_record/errors.rb#L354-L364) in it # Additional Notes I've looked at other places where API consumers submit raw strings that we just pass in raw, and I cannot find other plausible places where this could be an issue # How to test the change? Bin_test updated to include a new line in the input to the unhandled exception API Co-authored-by: gyuheon.oh <gyuheon.oh@datadoghq.com>
1 parent f7427db commit 02d95c0

4 files changed

Lines changed: 13 additions & 4 deletions

File tree

bin_tests/src/bin/crashtracker_bin_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ mod unix {
177177

178178
crashtracker::report_unhandled_exception(
179179
Some("RuntimeException"),
180-
Some("an exception occured"),
180+
Some("\n an exception \n occured \n"),
181181
stacktrace,
182182
)?;
183183

bin_tests/tests/crashtracker_bin_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ fn test_crash_tracking_bin_unhandled_exception() {
138138
PayloadValidator::new(payload)
139139
.validate_counters()?
140140
.validate_error_kind("UnhandledException")?
141-
.validate_error_message_contains("Process was terminated due to an unhandled exception of type 'RuntimeException'. Message: an exception occured")?
141+
.validate_error_message_contains("Process was terminated due to an unhandled exception of type 'RuntimeException'. Message: \n an exception \n occured \n")?
142142
// The two frames emitted in the bin: test_function1 and test_function2
143143
.validate_callstack_functions(&["test_function1", "test_function2"])?;
144144

libdd-crashtracker/src/collector/crash_handler.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,15 @@ pub fn report_unhandled_exception(
421421
let tid = libdd_common::threading::get_current_thread_id() as libc::pid_t;
422422

423423
let error_type_str = exception_type.unwrap_or("<unknown>");
424-
let error_message_str = exception_message.unwrap_or("<no message>");
424+
425+
// This allocates but this is okay because we are not in a signal handler.
426+
// This is necessary, because user defined exception messages can have newlines
427+
// and the receiver state machine parsing depends on newlines for different sections
428+
// of the crash report.
429+
let error_message_str = exception_message
430+
.unwrap_or("<no message>")
431+
.replace('\n', "\\n")
432+
.replace('\r', "\\r");
425433
let message = format!(
426434
"Process was terminated due to an unhandled exception of type '{error_type_str}'. \
427435
Message: {error_message_str}"

libdd-crashtracker/src/receiver/receive_report.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,8 @@ fn process_line(
268268

269269
StdinState::Message if line.starts_with(DD_CRASHTRACK_END_MESSAGE) => StdinState::Waiting,
270270
StdinState::Message => {
271-
builder.with_message(line.to_string())?;
271+
let unescaped = line.replace("\\n", "\n").replace("\\r", "\r");
272+
builder.with_message(unescaped)?;
272273
StdinState::Message
273274
}
274275

0 commit comments

Comments
 (0)