Skip to content

Commit

Permalink
fix(processor): Remove form data envelope items (#4428)
Browse files Browse the repository at this point in the history
  • Loading branch information
iambriccardo authored Jan 7, 2025
1 parent 2fafc92 commit ad26c42
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion relay-server/src/services/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ mod session;
mod span;
pub use span::extract_transaction_span;

mod standalone;
#[cfg(feature = "processing")]
mod unreal;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(),
Expand Down
14 changes: 14 additions & 0 deletions relay-server/src/services/processor/standalone.rs
Original file line number Diff line number Diff line change
@@ -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<StandaloneGroup>) {
managed_envelope.retain_items(|i| match i.ty() {
ItemType::FormData => ItemAction::DropSilently,
_ => ItemAction::Keep,
});
}
4 changes: 2 additions & 2 deletions relay-server/src/utils/multipart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
42 changes: 42 additions & 0 deletions tests/integration/test_attachments.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

0 comments on commit ad26c42

Please sign in to comment.