Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(replays): Use all bytes after the first newline split as the body value #1682

Merged
merged 9 commits into from
Dec 9, 2022
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**:

Expand Down
130 changes: 114 additions & 16 deletions relay-replays/src/recording.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,24 @@ use serde::{Deserialize, Serialize};
use serde_json::{Error, Value};

pub fn process_recording(bytes: &[u8]) -> Result<Vec<u8>, 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)?;
Expand Down Expand Up @@ -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");
Expand Down