Skip to content

Commit 18baf91

Browse files
authored
fix(minidump): Streaming uploads from external relays (#5977)
1 parent 7f5f1f1 commit 18baf91

8 files changed

Lines changed: 204 additions & 95 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
- Obtain PII values for `SpanData` fields from `sentry-conventions`. ([#5997](https://github.com/getsentry/relay/pull/5997))
1313
- Add `sentry.dsc.transaction` and `sentry.dsc.trace_id` to all spans. ([#6001](https://github.com/getsentry/relay/pull/6001), [#6004](https://github.com/getsentry/relay/pull/6004), [#6008](https://github.com/getsentry/relay/pull/6008))
1414

15+
**Internal**:
16+
17+
- Always allow `Upload-Defer-Length: 1` on the `/upload` endpoint. ([#5977](https://github.com/getsentry/relay/pull/5977))
18+
1519
## 26.5.0
1620

1721
**Features**:

relay-server/src/endpoints/upload.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ use crate::service::ServiceState;
3030
#[cfg(feature = "processing")]
3131
use crate::services::objectstore;
3232
use crate::services::projects::cache::Project;
33-
use crate::services::upload::{self, ByteStream, SignedLocation};
33+
use crate::services::upload::{
34+
self, ByteStream, Final, LocationQueryParams, Provisional, SignedLocation, UploadLength,
35+
};
3436
use crate::services::upstream::UpstreamRequestError;
3537
use crate::statsd::RelayCounters;
3638
use crate::utils::{ApiErrorResponse, MeteredStream};
@@ -76,7 +78,6 @@ impl IntoResponse for Error {
7678
}
7779

7880
let status = match self {
79-
Error::Tus(tus::Error::DeferLengthNotAllowed) => StatusCode::FORBIDDEN,
8081
Error::Tus(_) => StatusCode::BAD_REQUEST,
8182
Error::Request(error) => return error.into_response(),
8283
Error::SendError(_) => StatusCode::INTERNAL_SERVER_ERROR,
@@ -129,7 +130,7 @@ impl IntoResponse for Error {
129130
}
130131
}
131132

132-
impl IntoResponse for SignedLocation {
133+
impl<L: UploadLength> IntoResponse for SignedLocation<L> {
133134
fn into_response(self) -> Response {
134135
let mut headers = tus::response_headers();
135136
match self.into_header_value() {
@@ -153,8 +154,7 @@ async fn handle_post(
153154
check_kill_switch(&state)?;
154155

155156
relay_log::trace!("Validating headers");
156-
let upload_length = tus::validate_post_headers(&headers, meta.request_trust().is_trusted())
157-
.map_err(Error::from)?;
157+
let upload_length = tus::validate_post_headers(&headers).map_err(Error::from)?;
158158
let config = state.config();
159159

160160
if upload_length.is_some_and(|len| len > config.max_upload_size()) {
@@ -177,6 +177,7 @@ async fn handle_post(
177177
let scoping = check_request(&state, meta, upload_length, project).await?;
178178

179179
// Unconditionally create the upload location:
180+
relay_log::trace!("Creating upload location");
180181
let result = create(&state, scoping, upload_length).await;
181182
let location = result.inspect_err(|e| {
182183
relay_log::warn!(error = e as &dyn std::error::Error, "create failed");
@@ -195,7 +196,7 @@ async fn handle_patch(
195196
meta: RequestMeta,
196197
headers: HeaderMap,
197198
Path(upload::LocationPath { project_id, key }): Path<upload::LocationPath>,
198-
Query(upload::LocationQueryParams { length, signature }): Query<upload::LocationQueryParams>,
199+
Query(LocationQueryParams { length, signature }): Query<LocationQueryParams<Provisional>>,
199200
body: Body,
200201
) -> axum::response::Result<impl IntoResponse> {
201202
check_kill_switch(&state)?;
@@ -220,15 +221,15 @@ async fn handle_patch(
220221
})?;
221222

222223
relay_log::trace!("Checking request");
223-
let scoping = check_request(&state, meta, length, project).await?;
224+
let scoping = check_request(&state, meta, length.value(), project).await?;
224225

225226
let stream = body
226227
.into_data_stream()
227228
.map(|result| result.map_err(io::Error::other))
228229
.boxed();
229230
let stream = MeteredStream::new(stream, "upload");
230231

231-
let (lower_bound, upper_bound) = match length {
232+
let (lower_bound, upper_bound) = match length.value() {
232233
None => (1, config.max_upload_size()),
233234
Some(u) => (u, u),
234235
};
@@ -282,7 +283,7 @@ async fn create(
282283
state: &ServiceState,
283284
scoping: Scoping,
284285
upload_length: Option<usize>,
285-
) -> Result<SignedLocation, Error> {
286+
) -> Result<SignedLocation<Provisional>, Error> {
286287
let location = state
287288
.upload()
288289
.send(upload::Create {
@@ -297,9 +298,9 @@ async fn create(
297298
async fn upload(
298299
state: &ServiceState,
299300
scoping: Scoping,
300-
location: SignedLocation,
301+
location: SignedLocation<Provisional>,
301302
stream: BoundedStream<MeteredStream<ByteStream>>,
302-
) -> Result<SignedLocation, Error> {
303+
) -> Result<SignedLocation<Final>, Error> {
303304
let location = state
304305
.upload()
305306
.send(upload::Stream {

relay-server/src/processing/utils/attachments.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,23 +40,22 @@ fn validate(item: &Item, config: &Config) -> Result<(), ProcessingError> {
4040

4141
#[cfg(feature = "processing")]
4242
{
43+
use crate::services::upload::{Final, SignedLocation};
44+
4345
if !item.is_attachment_ref() {
4446
return Ok(());
4547
}
4648

4749
let payload = item.payload();
4850
let payload: AttachmentPlaceholder =
4951
serde_json::from_slice(&payload).map_err(|_| ProcessingError::InvalidAttachmentRef)?;
50-
let signed_location =
51-
crate::services::upload::SignedLocation::try_from_str(payload.location)
52-
.ok_or(ProcessingError::InvalidAttachmentRef)?;
52+
let signed_location: SignedLocation<Final> = SignedLocation::try_from_str(payload.location)
53+
.ok_or(ProcessingError::InvalidAttachmentRef)?;
5354
// NOTE: Using the received timestamp here breaks tests without a pop-relay.
5455
let location = signed_location
5556
.verify(chrono::Utc::now(), config)
5657
.map_err(|_| ProcessingError::InvalidAttachmentRef)?;
57-
let signed_length = location
58-
.length
59-
.ok_or(ProcessingError::InvalidAttachmentRef)?;
58+
let signed_length = location.length.into_inner();
6059

6160
match item.attachment_body_size() == signed_length {
6261
true => Ok(()),

relay-server/src/services/store.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ use crate::metrics::{ArrayEncoding, BucketEncoder, MetricOutcomes};
3939
use crate::service::ServiceError;
4040
use crate::services::global_config::GlobalConfigHandle;
4141
use crate::services::outcome::{DiscardReason, Outcome, TrackOutcome};
42-
use crate::services::upload::SignedLocation;
42+
use crate::services::upload::{Final, SignedLocation};
4343
use crate::statsd::{RelayCounters, RelayGauges, RelayTimers};
4444
use crate::utils::{self, FormDataIter};
4545

@@ -1031,7 +1031,7 @@ impl StoreService {
10311031
let payload = item.payload();
10321032
let placeholder: AttachmentPlaceholder<'_> =
10331033
serde_json::from_slice(&payload).map_err(|_| StoreError::InvalidAttachmentRef)?;
1034-
let location = SignedLocation::try_from_str(placeholder.location)
1034+
let location = SignedLocation::<Final>::try_from_str(placeholder.location)
10351035
.ok_or(StoreError::InvalidAttachmentRef)?
10361036
.verify(Utc::now(), &self.config)
10371037
.map_err(|_| StoreError::InvalidAttachmentRef)?;

0 commit comments

Comments
 (0)