From 3608f046485bf28ece3c1d6dde529736fad7e577 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 1 Sep 2026 10:32:10 +0200 Subject: [PATCH 1/6] remove `CompressedMinidump` counter --- relay-server/src/endpoints/minidump.rs | 4 ---- relay-server/src/statsd.rs | 3 --- 2 files changed, 7 deletions(-) diff --git a/relay-server/src/endpoints/minidump.rs b/relay-server/src/endpoints/minidump.rs index d59997d47a5..1e2f085e72c 100644 --- a/relay-server/src/endpoints/minidump.rs +++ b/relay-server/src/endpoints/minidump.rs @@ -32,7 +32,6 @@ use crate::service::ServiceState; use crate::services::outcome::{DiscardAttachmentType, DiscardItemType, DiscardReason, Outcome}; use crate::services::projects::project::ProjectState; use crate::services::upload::{ByteStream, ProjectContext, Upload}; -use crate::statsd::RelayCounters; use crate::utils::{ self, AttachmentStrategy, SizeSplit, find_error_source, is_length_limit_error, peek_n, read_bytes_into_item, read_field_into_item, @@ -190,9 +189,6 @@ fn decode_minidump(minidump_data: Bytes, max_size: usize) -> Result "logs.envelope", RelayCounters::ProfileChunksWithoutPlatform => "profile_chunk.no_platform", RelayCounters::ErrorProcessed => "event.error.processed", - RelayCounters::CompressedMinidump => "minidump.compressed.count", RelayCounters::TraceMetricNilTraceId => "trace_metric.nil_trace_id", } } From a6e1ce00e1a49c2bf35deaba2401299bdae7c338 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 1 Sep 2026 11:02:08 +0200 Subject: [PATCH 2/6] replace sync decoder with existing async decoder --- relay-server/src/endpoints/minidump.rs | 107 ++++++------------------- 1 file changed, 26 insertions(+), 81 deletions(-) diff --git a/relay-server/src/endpoints/minidump.rs b/relay-server/src/endpoints/minidump.rs index 1e2f085e72c..431d12d7724 100644 --- a/relay-server/src/endpoints/minidump.rs +++ b/relay-server/src/endpoints/minidump.rs @@ -2,10 +2,7 @@ use axum::extract::{DefaultBodyLimit, Request}; use axum::response::IntoResponse; use axum::routing::{MethodRouter, post}; use bytes::Bytes; -use bzip2::read::BzDecoder; -use flate2::read::GzDecoder; use futures::{self, Stream, StreamExt, TryStreamExt}; -use liblzma::read::XzDecoder; use multer::{Field, Multipart}; use relay_config::Config; use relay_dynamic_config::Feature; @@ -15,12 +12,9 @@ use relay_system::Addr; use smallvec::smallvec; use std::convert::Infallible; use std::error::Error; -use std::io::Cursor; -use std::io::Read; use tokio::io::BufReader; use tokio_util::io::{ReaderStream, StreamReader}; use tower_http::limit::RequestBodyLimitLayer; -use zstd::stream::Decoder as ZstdDecoder; use crate::constants::{ITEM_NAME_BREADCRUMBS1, ITEM_NAME_BREADCRUMBS2, ITEM_NAME_EVENT}; use crate::endpoints::common::{self, BadStoreRequest, TextResponse, upload_stream}; @@ -126,16 +120,6 @@ fn validate_minidump(data: &[u8]) -> Result<(), BadStoreRequest> { Ok(()) } -/// Convenience wrapper to let a decoder decode its full input into a buffer. -/// -/// Stops reading once `max_size` is exceeded and returns an error. This prevents -/// decompression bombs from exhausting memory. -fn run_decoder(mut decoder: impl Read) -> std::io::Result> { - let mut buffer = Vec::new(); - decoder.read_to_end(&mut buffer)?; - Ok(buffer) -} - /// Types of compression we support for minidump payloads. enum Compression { None, @@ -161,43 +145,23 @@ impl Compression { } } -/// Creates a decoder based on the magic bytes in the minidump payload. -fn decoder_from(minidump_data: Bytes) -> Option> { - match Compression::from(&minidump_data) { - Compression::None => None, - Compression::Gzip => Some(Box::new(GzDecoder::new(Cursor::new(minidump_data)))), - Compression::Xz => Some(Box::new(XzDecoder::new(Cursor::new(minidump_data)))), - Compression::Bzip2 => Some(Box::new(BzDecoder::new(Cursor::new(minidump_data)))), - Compression::Zstd => match ZstdDecoder::new(Cursor::new(minidump_data)) { - Ok(decoder) => Some(Box::new(decoder)), - Err(ref err) => { - relay_log::error!(error = err as &dyn Error, "failed to create ZstdDecoder"); - None - } - }, - } -} - /// Tries to decode a minidump using any of the supported compression formats /// or returns the provided minidump payload untouched if no format where detected. /// /// Returns an `Overflow` error if the decompressed size exceeds `max_size`. -fn decode_minidump(minidump_data: Bytes, max_size: usize) -> Result { - let Some(decoder) = decoder_from(minidump_data.clone()) else { - // this means we haven't detected any compression container - // proceed to process the payload untouched (as a plain minidump). - return Ok(minidump_data); - }; - - let decoder = decoder.take(max_size.saturating_add(1) as u64); - - match run_decoder(decoder) { - Ok(decoded) => { - if decoded.len() > max_size { - let item_type = DiscardItemType::Attachment(DiscardAttachmentType::Minidump); - return Err(BadStoreRequest::ItemTooLarge(item_type)); - } - Ok(Bytes::from(decoded)) +async fn decode_minidump(minidump_data: Bytes, max_size: usize) -> Result { + let stream = futures::stream::once(async move { Ok::<_, Infallible>(minidump_data) }); + let decoded = decode_stream(stream) + .await + .map_err(BadStoreRequest::InvalidCompression)?; + + // Decoding happens lazily while peeking, stopping early once `max_size` is + // exceeded. This prevents decompression bombs from exhausting memory. + match utils::stream::split_by_size(decoded, max_size).await { + Ok(SizeSplit::Small(decoded)) => Ok(decoded), + Ok(SizeSplit::Large(_)) => { + let item_type = DiscardItemType::Attachment(DiscardAttachmentType::Minidump); + Err(BadStoreRequest::ItemTooLarge(item_type)) } Err(err) => { // we detected a compression container but failed to decode it @@ -467,7 +431,9 @@ async fn multipart_to_items( .await .reject(&items)? .unwrap_or(payload); - let payload = decode_minidump(payload, config.max_attachment_size()).reject(&items)?; + let payload = decode_minidump(payload, config.max_attachment_size()) + .await + .reject(&items)?; items.try_modify(|items, records| -> Result<(), BadStoreRequest> { let minidump_item = items @@ -582,8 +548,10 @@ async fn raw_minidump_to_item( .map_err(|e| BadStoreRequest::InvalidBody(std::io::Error::other(e)))? { SizeSplit::Small(bytes) => { + let payload = decode_minidump(bytes, state.config().max_attachment_size()) + .await + .reject(&item)?; item.try_modify(|inner, records| -> Result<(), BadStoreRequest> { - let payload = decode_minidump(bytes, state.config().max_attachment_size())?; inner.set_payload(ContentType::Minidump, payload); records.lenient(DataCategory::Attachment); // decoding changes its size validate_minidump(&inner.payload())?; @@ -617,8 +585,10 @@ async fn raw_minidump_to_item( None => BadStoreRequest::InvalidBody(std::io::Error::other(e)), })?; + let payload = decode_minidump(minidump_data, state.config().max_attachment_size()) + .await + .reject(&item)?; item.try_modify(|inner, records| -> Result<(), BadStoreRequest> { - let payload = decode_minidump(minidump_data, state.config().max_attachment_size())?; inner.set_payload(ContentType::Minidump, payload); records.lenient(DataCategory::Attachment); // decoding the minidump changes its size validate_minidump(&inner.payload())?; @@ -784,31 +754,6 @@ mod tests { Ok(Bytes::from(compressed)) } - #[test] - fn test_validate_encoded_minidump() -> Result<(), Box> { - let encoders: Vec = vec![encode_gzip, encode_zst, encode_bzip, encode_xz]; - for encoder in &encoders { - let be_minidump = b"PMDMxxxxxx"; - let compressed = encoder(be_minidump)?; - let decoder = decoder_from(compressed).unwrap(); - assert!(run_decoder(decoder).is_ok()); - - let le_minidump = b"MDMPxxxxxx"; - let compressed = encoder(le_minidump)?; - let decoder = decoder_from(compressed).unwrap(); - assert!(run_decoder(decoder).is_ok()); - - let garbage = b"xxxxxx"; - let compressed = encoder(garbage)?; - let decoder = decoder_from(compressed).unwrap(); - let decoded = run_decoder(decoder); - assert!(decoded.is_ok()); - assert!(validate_minidump(&decoded.unwrap()).is_err()); - } - - Ok(()) - } - fn stream_of(data: Bytes) -> impl Stream> + Send + 'static { futures::stream::once(async move { Ok(data) }) } @@ -866,19 +811,19 @@ mod tests { Ok(()) } - #[test] - fn test_decode_minidump_size_limit() -> Result<(), Box> { + #[tokio::test] + async fn test_decode_minidump_size_limit() -> Result<(), Box> { // Create a minidump that will decompress to 100 bytes let minidump_data = b"xxxxxxxxxx".repeat(10); let compressed = encode_gzip(&minidump_data)?; // With a limit larger than the decompressed size, decoding should succeed - let result = decode_minidump(compressed.clone(), 200); + let result = decode_minidump(compressed.clone(), 200).await; assert!(result.is_ok()); assert_eq!(result.unwrap().len(), 100); // With a limit smaller than the decompressed size, decoding should fail with Overflow - let result = decode_minidump(compressed, 50); + let result = decode_minidump(compressed, 50).await; assert!(matches!(result, Err(BadStoreRequest::ItemTooLarge(_)))); Ok(()) From afcce829bf5e002b580c715bf9bc7f9021eb6296 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 1 Sep 2026 11:05:22 +0200 Subject: [PATCH 3/6] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e46f9e2611..65864134620 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - Reshape Nintendo Switch crashes so the issue title falls back to the crashing function instead of the raw abort result code, and raise their level to `fatal`. ([#6253](https://github.com/getsentry/relay/pull/6253)) - Reject Nintendo Switch dying message attachments with an invalid magic number instead of panicking on a short payload. ([#6253](https://github.com/getsentry/relay/pull/6253)) - Downgrade Kafka to prevent producers from getting stuck. ([#6336](https://github.com/getsentry/relay/pull/6336)) +- Use async instead of sync decompression in minidump endpoint. ([#6341](https://github.com/getsentry/relay/pull/6341)) **Internal**: From d41f6f4648f8be17f1dc1602f67e26d01ffb6d3c Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 1 Sep 2026 11:37:32 +0200 Subject: [PATCH 4/6] add early return if no compression --- relay-server/src/endpoints/minidump.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/relay-server/src/endpoints/minidump.rs b/relay-server/src/endpoints/minidump.rs index 431d12d7724..e035fe10258 100644 --- a/relay-server/src/endpoints/minidump.rs +++ b/relay-server/src/endpoints/minidump.rs @@ -150,6 +150,9 @@ impl Compression { /// /// Returns an `Overflow` error if the decompressed size exceeds `max_size`. async fn decode_minidump(minidump_data: Bytes, max_size: usize) -> Result { + if matches!(Compression::from(&minidump_data), Compression::None) { + return Ok(minidump_data); + } let stream = futures::stream::once(async move { Ok::<_, Infallible>(minidump_data) }); let decoded = decode_stream(stream) .await From 28e1d50fe0cf867a08a3b26c0a737dc2743cdd10 Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 1 Sep 2026 11:38:18 +0200 Subject: [PATCH 5/6] fix off by one --- relay-server/src/endpoints/minidump.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/endpoints/minidump.rs b/relay-server/src/endpoints/minidump.rs index e035fe10258..b66b6d4365e 100644 --- a/relay-server/src/endpoints/minidump.rs +++ b/relay-server/src/endpoints/minidump.rs @@ -160,7 +160,7 @@ async fn decode_minidump(minidump_data: Bytes, max_size: usize) -> Result Ok(decoded), Ok(SizeSplit::Large(_)) => { let item_type = DiscardItemType::Attachment(DiscardAttachmentType::Minidump); From 6c6794d8a80796322a0157d1b48f62535f6daeec Mon Sep 17 00:00:00 2001 From: tobias-wilfert Date: Tue, 1 Sep 2026 13:10:32 +0200 Subject: [PATCH 6/6] ref --- relay-server/src/endpoints/minidump.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/relay-server/src/endpoints/minidump.rs b/relay-server/src/endpoints/minidump.rs index b66b6d4365e..4aca2bc7a51 100644 --- a/relay-server/src/endpoints/minidump.rs +++ b/relay-server/src/endpoints/minidump.rs @@ -158,8 +158,6 @@ async fn decode_minidump(minidump_data: Bytes, max_size: usize) -> Result Ok(decoded), Ok(SizeSplit::Large(_)) => {