From ad26c42784296739f063e5a5e4ec6a3e32a07c5c Mon Sep 17 00:00:00 2001 From: Riccardo Busetti Date: Tue, 7 Jan 2025 16:10:16 +0100 Subject: [PATCH] fix(processor): Remove form data envelope items (#4428) --- CHANGELOG.md | 1 + relay-server/src/services/processor.rs | 5 ++- .../src/services/processor/standalone.rs | 14 +++++++ relay-server/src/utils/multipart.rs | 4 +- tests/integration/test_attachments.py | 42 +++++++++++++++++++ 5 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 relay-server/src/services/processor/standalone.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b6cadba938..7e4888d080a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Remove the `spool` command from Relay. ([#4423](https://github.com/getsentry/relay/pull/4423)) - Bump `sentry-native` to `0.7.17` and remove cross compilation in CI. ([#4427](https://github.com/getsentry/relay/pull/4427)) +- Remove `form_data` envelope items from standalone envelopes. ([#4428](https://github.com/getsentry/relay/pull/4428)) ## 24.12.1 diff --git a/relay-server/src/services/processor.rs b/relay-server/src/services/processor.rs index e75d5f96a4f..29c4f1fe7c9 100644 --- a/relay-server/src/services/processor.rs +++ b/relay-server/src/services/processor.rs @@ -94,6 +94,7 @@ mod session; mod span; pub use span::extract_transaction_span; +mod standalone; #[cfg(feature = "processing")] mod unreal; @@ -309,7 +310,7 @@ impl ProcessingGroup { )) } - // Exract all metric items. + // Extract all metric items. // // Note: Should only be relevant in proxy mode. In other modes we send metrics through // a separate pipeline. @@ -1857,6 +1858,8 @@ impl EnvelopeProcessorService { #[allow(unused_mut)] let mut extracted_metrics = ProcessingExtractedMetrics::new(); + standalone::process(managed_envelope); + profile::filter( managed_envelope, &Annotated::empty(), diff --git a/relay-server/src/services/processor/standalone.rs b/relay-server/src/services/processor/standalone.rs new file mode 100644 index 00000000000..aceb0294410 --- /dev/null +++ b/relay-server/src/services/processor/standalone.rs @@ -0,0 +1,14 @@ +use crate::envelope::ItemType; +use crate::services::processor::StandaloneGroup; +use crate::utils::{ItemAction, TypedEnvelope}; + +/// Processes a standalone envelope by removing unnecessary items. +/// +/// This function removes form data items from the envelope since they are not allowed in +/// standalone processing. +pub fn process(managed_envelope: &mut TypedEnvelope) { + managed_envelope.retain_items(|i| match i.ty() { + ItemType::FormData => ItemAction::DropSilently, + _ => ItemAction::Keep, + }); +} diff --git a/relay-server/src/utils/multipart.rs b/relay-server/src/utils/multipart.rs index 990d41f3099..fc753924f09 100644 --- a/relay-server/src/utils/multipart.rs +++ b/relay-server/src/utils/multipart.rs @@ -184,8 +184,8 @@ where let form_data = form_data.into_inner(); if !form_data.is_empty() { let mut item = Item::new(ItemType::FormData); - // Content type is Text (since it is not a json object but multiple - // json arrays serialized one after the other. + // Content type is `Text` (since it is not a json object but multiple + // json arrays serialized one after the other). item.set_payload(ContentType::Text, form_data); items.push(item); } diff --git a/tests/integration/test_attachments.py b/tests/integration/test_attachments.py index 408bde93e53..a61979c1ccf 100644 --- a/tests/integration/test_attachments.py +++ b/tests/integration/test_attachments.py @@ -371,3 +371,45 @@ def test_event_with_attachment( _, event = attachments_consumer.get_event() assert event["event_id"] == event_id + + +def test_form_data_is_rejected( + mini_sentry, relay_with_processing, attachments_consumer, outcomes_consumer +): + """ + Test that form data entries (those without filenames) are rejected and generate outcomes. + """ + project_id = 42 + event_id = "515539018c9b4260a6f999572f1661ee" + + mini_sentry.add_full_project_config(project_id) + relay = relay_with_processing() + attachments_consumer = attachments_consumer() + + # Create attachments with both file content and form data + attachments = [ + ("att_1", "foo.txt", b"file content"), # Valid file attachment + ("form_key", None, b"form value"), # Form data that should be rejected + ] + + relay.send_attachments(project_id, event_id, attachments) + + # Check that only the file attachment was processed + attachment = attachments_consumer.get_individual_attachment() + assert attachment["attachment"].pop("id") # ID is random + assert attachment == { + "type": "attachment", + "attachment": { + "attachment_type": "event.attachment", + "chunks": 0, + "data": b"file content", + "name": "foo.txt", + "size": len(b"file content"), + "rate_limited": False, + }, + "event_id": event_id, + "project_id": project_id, + } + + # Verify no more attachments were processed + attachments_consumer.assert_empty()