diff --git a/CHANGELOG.md b/CHANGELOG.md index 07ceb3a9b1..ea50350c35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Make `attachment_type` on envelope items forward compatible by adding fallback variant. ([#1638](https://github.com/getsentry/relay/pull/1638)) - Relay no longer accepts transaction events older than 5 days. Previously the event was accepted and stored, but since metrics for such old transactions are not supported it did not show up in parts of Sentry such as the Performance landing page. ([#1663](https://github.com/getsentry/relay/pull/1663)) - Apply dynamic sampling to transactions from older SDKs and even in case Relay cannot load project information. This avoids accidentally storing 100% of transactions. ([#1667](https://github.com/getsentry/relay/pull/1667)) +- Replay recording parser now uses the entire body rather than a subset. ([#1682](https://github.com/getsentry/relay/pull/1682)) **Internal**: diff --git a/relay-replays/src/recording.rs b/relay-replays/src/recording.rs index 0f51de9e16..a858821d65 100644 --- a/relay-replays/src/recording.rs +++ b/relay-replays/src/recording.rs @@ -17,17 +17,24 @@ use serde::{Deserialize, Serialize}; use serde_json::{Error, Value}; pub fn process_recording(bytes: &[u8]) -> Result, RecordingParseError> { - // Split recording headers and body. - let mut x = bytes.split(|b| b == &b'\n'); - let header = x.next().ok_or_else(|| { - RecordingParseError::Message("no headers found. was data provided?".to_string()) - })?; - let body = x.next().ok_or_else(|| { - RecordingParseError::Message("no data found. are the headers missing?".to_string()) - })?; + // Check for null byte condition. + if bytes.is_empty() { + return Err(RecordingParseError::Message("no data found.".to_string())); + } + + // Find the header value. + let header = bytes + .split(|b| b == &b'\n') + .next() + .ok_or_else(|| RecordingParseError::Message("no headers found.".to_string()))?; + + // Find the body value. + if bytes.len() <= header.len() + 1 { + return Err(RecordingParseError::Message("no body found.".to_string())); + } // Deserialization. - let mut events = loads(body)?; + let mut events = loads(&bytes[header.len() + 1..])?; // Processing. strip_pii(&mut events).map_err(RecordingParseError::ProcessingAction)?; @@ -564,20 +571,111 @@ mod tests { serde_json::from_slice(bytes) } - // RRWeb Payload Coverage + // End to end test coverage. #[test] - fn test_process_recording_no_config() { - let payload = include_bytes!("../tests/fixtures/rrweb-binary.txt"); - recording::process_recording(payload).unwrap(); + fn test_process_recording_end_to_end() { + // Valid compressed rrweb payload. Contains a 16 byte header followed by a new line + // character and concludes with a gzipped rrweb payload. + let payload: [u8; 241] = [ + 123, 34, 115, 101, 103, 109, 101, 110, 116, 95, 105, 100, 34, 58, 51, 125, 10, 120, + 156, 149, 144, 91, 106, 196, 32, 20, 64, 247, 114, 191, 237, 160, 241, 145, 234, 38, + 102, 1, 195, 124, 152, 104, 6, 33, 169, 193, 40, 52, 4, 247, 94, 91, 103, 40, 20, 108, + 59, 191, 247, 30, 207, 225, 122, 57, 32, 238, 171, 5, 69, 17, 24, 29, 53, 168, 3, 54, + 159, 194, 88, 70, 4, 193, 234, 55, 23, 157, 127, 219, 64, 93, 14, 120, 7, 37, 100, 1, + 119, 80, 29, 102, 8, 156, 1, 213, 11, 4, 209, 45, 246, 60, 77, 155, 141, 160, 94, 232, + 43, 206, 232, 206, 118, 127, 176, 132, 177, 7, 203, 42, 75, 36, 175, 44, 231, 63, 88, + 217, 229, 107, 174, 179, 45, 234, 101, 45, 172, 232, 49, 163, 84, 22, 191, 232, 63, 61, + 207, 93, 130, 229, 189, 216, 53, 138, 84, 182, 139, 178, 199, 191, 22, 139, 179, 238, + 196, 227, 244, 134, 137, 240, 158, 60, 101, 34, 255, 18, 241, 6, 116, 42, 212, 119, 35, + 234, 27, 40, 24, 130, 213, 102, 12, 105, 25, 160, 252, 147, 222, 103, 175, 205, 215, + 182, 45, 168, 17, 48, 118, 210, 105, 142, 229, 217, 168, 163, 189, 249, 80, 254, 19, + 146, 59, 13, 115, 10, 144, 115, 190, 126, 0, 2, 68, 180, 16, + ]; + + let result = recording::process_recording(&payload); + match result { + Ok(v) => assert!(!v.is_empty()), + Err(_) => unreachable!(), + } } #[test] - fn test_process_recording_has_config() { - let payload = include_bytes!("../tests/fixtures/rrweb-binary.txt"); - recording::process_recording(payload).unwrap(); + fn test_process_recording_no_body_data() { + // Empty bodies can not be decompressed and fail. + let payload: [u8; 17] = [ + 123, 34, 115, 101, 103, 109, 101, 110, 116, 95, 105, 100, 34, 58, 51, 125, 10, + ]; + + let result = recording::process_recording(&payload); + match result { + Ok(_) => unreachable!(), + Err(e) => match e { + recording::RecordingParseError::Message(er) => { + assert_eq!(er, "no body found.".to_string()) + } + _ => unreachable!(), + }, + } + } + + #[test] + fn test_process_recording_bad_body_data() { + // Invalid gzip body contents. Can not deflate. + let payload: [u8; 18] = [ + 123, 34, 115, 101, 103, 109, 101, 110, 116, 95, 105, 100, 34, 58, 51, 125, 10, 22, + ]; + + let result = recording::process_recording(&payload); + match result { + Ok(_) => unreachable!(), + Err(e) => match e { + recording::RecordingParseError::IoError(er) => { + assert_eq!(er.to_string(), "corrupt deflate stream".to_string()) + } + _ => unreachable!(), + }, + } } + #[test] + fn test_process_recording_no_headers() { + // No header delimiter. Entire payload is consumed as headers. The empty body fails. + let payload: [u8; 16] = [ + 123, 34, 115, 101, 103, 109, 101, 110, 116, 95, 105, 100, 34, 58, 51, 125, + ]; + + let result = recording::process_recording(&payload); + match result { + Ok(_) => unreachable!(), + Err(e) => match e { + recording::RecordingParseError::Message(er) => { + assert_eq!(er, "no body found.".to_string()) + } + _ => unreachable!(), + }, + } + } + + #[test] + fn test_process_recording_no_contents() { + // Empty payload can not be decompressed. Header check never fails. + let payload: [u8; 0] = []; + + let result = recording::process_recording(&payload); + match result { + Ok(_) => unreachable!(), + Err(e) => match e { + recording::RecordingParseError::Message(er) => { + assert_eq!(er, "no data found.".to_string()) + } + _ => unreachable!(), + }, + } + } + + // RRWeb Payload Coverage + #[test] fn test_pii_credit_card_removal() { let payload = include_bytes!("../tests/fixtures/rrweb-pii.json");