From 22c087b6fff48507c81d4369229e6db61c6b083a Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Wed, 10 Jun 2026 21:08:19 +0200 Subject: [PATCH 01/11] wip --- relay-server/src/endpoints/upload.rs | 12 ++- relay-server/src/services/upload.rs | 130 +++++++++++++++++++++------ 2 files changed, 114 insertions(+), 28 deletions(-) diff --git a/relay-server/src/endpoints/upload.rs b/relay-server/src/endpoints/upload.rs index 8e3640d9438..22330d53c1f 100644 --- a/relay-server/src/endpoints/upload.rs +++ b/relay-server/src/endpoints/upload.rs @@ -102,6 +102,7 @@ impl IntoResponse for Error { upload::Error::InvalidLocation(_) | upload::Error::SigningFailed => { StatusCode::INTERNAL_SERVER_ERROR } + upload::Error::SerializeFailed(_) => StatusCode::INTERNAL_SERVER_ERROR, upload::Error::InvalidSignature => StatusCode::BAD_REQUEST, upload::Error::ObjectstoreServiceUnavailable(_) => StatusCode::SERVICE_UNAVAILABLE, #[cfg(feature = "processing")] @@ -200,7 +201,11 @@ async fn handle_patch( meta: RequestMeta, headers: HeaderMap, Path(upload::LocationPath { project_id, key }): Path, - Query(LocationQueryParams { length, signature }): Query>, + Query(LocationQueryParams { + upload_length, + upload_signature, + other, + }): Query>, body: Body, ) -> axum::response::Result { check_kill_switch(&state)?; @@ -208,7 +213,8 @@ async fn handle_patch( relay_log::trace!("Validating headers"); tus::validate_patch_headers(&headers).map_err(Error::from)?; - let location = SignedLocation::from_parts(project_id, key, length, signature); + let location = + SignedLocation::from_parts(project_id, key, upload_length, upload_signature, other); let config = state.config(); @@ -233,7 +239,7 @@ async fn handle_patch( .boxed(); let stream = MeteredStream::new(stream, "upload"); - let (lower_bound, upper_bound) = match length.value() { + let (lower_bound, upper_bound) = match upload_length.value() { None => (1, config.max_upload_size()), Some(u) => (u, u), }; diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index 0b39761cdaf..aea445b1d98 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -1,5 +1,7 @@ //! Utilities for uploading large files. +use std::borrow::Cow; +use std::collections::BTreeMap; use std::fmt; use std::sync::Arc; use std::time::Duration; @@ -58,6 +60,8 @@ pub enum Error { Upstream(#[source] reqwest::Error), #[error("upstream provided invalid location: {0:?}")] InvalidLocation(Option), + #[error("serializing location failed: {0}")] + SerializeFailed(#[from] serde_urlencoded::ser::Error), #[error("failed to sign location")] SigningFailed, #[error("invalid signature")] @@ -82,6 +86,7 @@ impl Error { Error::Upstream(_) => "upstream_response", Error::InvalidLocation(_) => "invalid_location", Error::SigningFailed => "signing_failed", + Error::SerializeFailed(_) => "serialize_failed", Error::InvalidSignature => "invalid_signature", Error::ObjectstoreServiceUnavailable(_) => "service_unavailable", #[cfg(feature = "processing")] @@ -265,6 +270,7 @@ impl Service { project_id: scoping.project_id, key, length: Provisional(length), + other: Default::default(), } .try_sign(config) } @@ -291,6 +297,7 @@ impl Service { project_id, key, length, + other, } = location.verify(received, config)?; debug_assert_eq!(scoping.project_id, project_id); @@ -313,6 +320,7 @@ impl Service { project_id, key, length, + other, } .try_sign(config) } @@ -406,24 +414,33 @@ pub struct Location { pub key: String, /// Value of the `Upload-Length` header. `None` if `Upload-Defer-Length: 1`. pub length: L, + pub other: UploadParams, } impl Location { - fn as_uri(&self) -> String { + fn try_to_uri(&self) -> Result { let Location { project_id, key, length, + other, } = self; - match length.value() { - Some(length) => format!("/api/{project_id}/upload/{key}/?length={length}"), - None => format!("/api/{project_id}/upload/{key}/"), + let mut fields = other + .0 + .iter() + .map(|(k, v)| (k.as_str(), Cow::Borrowed(v))) + .collect::>(); + if let Some(length) = length.value() { + fields.push(("upload_length", Cow::Owned(length.to_string()))); } + + let query = serde_urlencoded::to_string(fields)?; + Ok(format!("/api/{project_id}/upload/{key}/?{query}")) } #[cfg(feature = "processing")] fn try_sign(self, config: &Config) -> Result, Error> { - let uri = self.as_uri(); + let uri = self.try_to_uri()?; let signature = config .credentials() .ok_or(Error::SigningFailed)? @@ -454,8 +471,51 @@ pub struct LocationPath { #[derive(Debug, Deserialize)] #[serde(bound = "L: UploadLength")] pub struct LocationQueryParams { - pub length: L, - pub signature: String, + #[serde(alias = "length")] + pub upload_length: L, + #[serde(alias = "signature")] + pub upload_signature: String, + #[serde(flatten)] + pub other: UploadParams, +} + +#[derive(Debug, Default)] +pub struct UploadParams(BTreeMap); + +impl<'de> Deserialize<'de> for UploadParams { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct UploadParamsVisitor; + + impl<'de> serde::de::Visitor<'de> for UploadParamsVisitor { + type Value = UploadParams; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("upload query parameters") + } + + fn visit_map(self, mut map: A) -> Result + where + A: serde::de::MapAccess<'de>, + { + let mut upload_params = BTreeMap::new(); + + while let Some(key) = map.next_key::<&str>()? { + if key.starts_with("upload_") { + upload_params.insert(key.to_owned(), map.next_value()?); + } else { + map.next_value::()?; + } + } + + Ok(UploadParams(upload_params)) + } + } + + deserializer.deserialize_map(UploadParamsVisitor) + } } /// A verifiable [`Location`] signed by this Relay or an upstream Relay. @@ -469,13 +529,19 @@ impl SignedLocation { /// Creates an unverified location from path and query params. /// /// Call `verify` to make sure the signature is correct. - pub fn from_parts(project_id: ProjectId, key: String, length: L, signature: String) -> Self { - // TODO: forward compat: allow other query params? + pub fn from_parts( + project_id: ProjectId, + key: String, + length: L, + signature: String, + other: UploadParams, + ) -> Self { Self { location: Location { project_id, key, length, + other, }, signature: Signature(signature), } @@ -483,23 +549,25 @@ impl SignedLocation { /// Converts the location into an URI for future reference. pub fn into_header_value(self) -> Result { - HeaderValue::from_str(&self.as_uri()).map_err(Error::Internal) + HeaderValue::from_str(&self.try_to_uri()?).map_err(Error::Internal) } - fn as_uri(&self) -> String { + fn try_to_uri(&self) -> Result { let Self { location, signature, } = self; - let mut uri = location.as_uri(); - uri.push(if location.length.value().is_some() { - '&' - } else { - '?' - }); // TODO: brittle. + let mut uri = location.try_to_uri()?; + uri.push( + if location.length.value().is_some() || !location.other.0.is_empty() { + '&' + } else { + '?' + }, + ); // TODO: brittle. uri.push_str("signature="); uri.push_str(&signature.to_string()); - uri + Ok(uri) } /// Converts the signed location into a location object. @@ -509,7 +577,7 @@ impl SignedLocation { pub fn verify(self, received: DateTime, config: &Config) -> Result, Error> { let public_key = config.public_key().ok_or(Error::SigningFailed)?; let is_valid = self.signature.verify( - self.location.as_uri().as_bytes(), + self.location.try_to_uri()?.as_bytes(), public_key, received, chrono::Duration::seconds(config.upload().max_age), @@ -559,9 +627,19 @@ where }; // Parse query parameters. - let LocationQueryParams { length, signature } = serde_urlencoded::from_str(query).ok()?; + let LocationQueryParams { + upload_length, + upload_signature, + other, + } = serde_urlencoded::from_str(query).ok()?; - Some(Self::from_parts(project_id, key, length, signature)) + Some(Self::from_parts( + project_id, + key, + upload_length, + upload_signature, + other, + )) } } @@ -657,7 +735,9 @@ impl UpstreamRequest for UploadRequest { let project_id = self.scoping.project_id; match &self.kind { RequestKind::Create { .. } => format!("/api/{project_id}/upload/"), - RequestKind::Upload { location, .. } => location.as_uri(), + RequestKind::Upload { location, .. } => location + .try_to_uri() + .expect("upload location should be serializable"), } .into() } @@ -756,7 +836,7 @@ mod tests { // Can only parse provisional: let provisional: LocationQueryParams = serde_urlencoded::from_str(url).unwrap(); - assert!(provisional.length.0.is_none()); + assert!(provisional.upload_length.0.is_none()); assert!(serde_urlencoded::from_str::>(url).is_err()); } @@ -767,8 +847,8 @@ mod tests { // Can only parse provisional: let provisional: LocationQueryParams = serde_urlencoded::from_str(json).unwrap(); - assert_eq!(provisional.length.0, Some(123)); + assert_eq!(provisional.upload_length.0, Some(123)); let full: LocationQueryParams = serde_urlencoded::from_str(json).unwrap(); - assert_eq!(full.length.0, 123); + assert_eq!(full.upload_length.0, 123); } } From eb52481650cccdc711dcfd73f97edc961b1175d6 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 11 Jun 2026 14:42:41 +0200 Subject: [PATCH 02/11] fix --- relay-server/src/services/upload.rs | 4 ++-- tests/integration/consts.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index aea445b1d98..353c76addb4 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -404,7 +404,7 @@ impl UploadLength for Final { /// The location can be converted into a URI to be put in the `Location` HTTP header /// used by the TUS protocol. /// -/// Calling [`Self::try_sign`] appends a `&signature=` query parameter that can later be used +/// Calling [`Self::try_sign`] appends an `&upload_signature=` query parameter that can later be used /// to validate whether the URI (especially the length) has been tempered with. #[derive(Debug)] pub struct Location { @@ -565,7 +565,7 @@ impl SignedLocation { '?' }, ); // TODO: brittle. - uri.push_str("signature="); + uri.push_str("upload_signature="); uri.push_str(&signature.to_string()); Ok(uri) } diff --git a/tests/integration/consts.py b/tests/integration/consts.py index 5846cbb5081..91d60f244da 100644 --- a/tests/integration/consts.py +++ b/tests/integration/consts.py @@ -2,6 +2,6 @@ METRICS_EXTRACTION_MIN_SUPPORTED_VERSION = 4 DUMMY_UPLOAD_PATH = "/api/42/upload/019cdc82ed6c7761ba21fd34b86481c2/" -DUMMY_UPLOAD_LOCATION = f"{DUMMY_UPLOAD_PATH}?length=11&signature=z_fUMhT0EZqJz6OQtwGHqTlOOLPpTVpvPa-rYTg18FVWZM1OGny-LeVJB5H-sSR_5e--I1xt-FlCmRG2bsmcAQ.eyJ0IjoiMjAyNi0wMy0xMVQxMDo0ODoxMy45NDM1ODNaIn0" +DUMMY_UPLOAD_LOCATION = f"{DUMMY_UPLOAD_PATH}?upload_length=11&upload_signature=z_fUMhT0EZqJz6OQtwGHqTlOOLPpTVpvPa-rYTg18FVWZM1OGny-LeVJB5H-sSR_5e--I1xt-FlCmRG2bsmcAQ.eyJ0IjoiMjAyNi0wMy0xMVQxMDo0ODoxMy45NDM1ODNaIn0" ZSTD_MAGIC_HEADER = b"\x28\xb5\x2f\xfd" From 90ec59e9bf938794dbea6e255ca0ec4a1469a85f Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 11 Jun 2026 21:07:28 +0200 Subject: [PATCH 03/11] fix: trailing ? --- relay-server/src/services/upload.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index 353c76addb4..fdf731633da 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -435,7 +435,10 @@ impl Location { } let query = serde_urlencoded::to_string(fields)?; - Ok(format!("/api/{project_id}/upload/{key}/?{query}")) + match query.as_str() { + "" => Ok(format!("/api/{project_id}/upload/{key}/")), + _ => Ok(format!("/api/{project_id}/upload/{key}/?{query}")), + } } #[cfg(feature = "processing")] @@ -557,17 +560,11 @@ impl SignedLocation { location, signature, } = self; - let mut uri = location.try_to_uri()?; - uri.push( - if location.length.value().is_some() || !location.other.0.is_empty() { - '&' - } else { - '?' - }, - ); // TODO: brittle. + let mut uri = dbg!(location.try_to_uri()?); + uri.push(if uri.ends_with('/') { '?' } else { '&' }); uri.push_str("upload_signature="); uri.push_str(&signature.to_string()); - Ok(uri) + Ok(dbg!(uri)) } /// Converts the signed location into a location object. From 66c5efbe47a32459a69f9cdf5e78d745a39195bd Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 11 Jun 2026 21:08:45 +0200 Subject: [PATCH 04/11] rm dbg --- relay-server/src/services/upload.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index fdf731633da..5ec73ed400b 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -560,11 +560,11 @@ impl SignedLocation { location, signature, } = self; - let mut uri = dbg!(location.try_to_uri()?); + let mut uri = location.try_to_uri()?; uri.push(if uri.ends_with('/') { '?' } else { '&' }); uri.push_str("upload_signature="); uri.push_str(&signature.to_string()); - Ok(dbg!(uri)) + Ok(uri) } /// Converts the signed location into a location object. From 74c2a6833f2ab344749d78e004b0f9eccae82c6e Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 11 Jun 2026 21:23:23 +0200 Subject: [PATCH 05/11] better serialization --- relay-server/src/endpoints/upload.rs | 6 ++--- relay-server/src/services/upload.rs | 39 +++++++++++++++------------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/relay-server/src/endpoints/upload.rs b/relay-server/src/endpoints/upload.rs index 22330d53c1f..bbd2b80e17b 100644 --- a/relay-server/src/endpoints/upload.rs +++ b/relay-server/src/endpoints/upload.rs @@ -31,7 +31,7 @@ use crate::service::ServiceState; use crate::services::objectstore; use crate::services::projects::cache::Project; use crate::services::upload::{ - self, ByteStream, Final, LocationQueryParams, Provisional, SignedLocation, UploadLength, + self, ByteStream, Final, Provisional, SignedLocation, SignedLocationQueryParams, UploadLength, }; use crate::services::upstream::UpstreamRequestError; use crate::statsd::RelayCounters; @@ -201,11 +201,11 @@ async fn handle_patch( meta: RequestMeta, headers: HeaderMap, Path(upload::LocationPath { project_id, key }): Path, - Query(LocationQueryParams { + Query(SignedLocationQueryParams { upload_length, upload_signature, other, - }): Query>, + }): Query>, body: Body, ) -> axum::response::Result { check_kill_switch(&state)?; diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index 5ec73ed400b..fe0d1ced1ef 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -1,6 +1,5 @@ //! Utilities for uploading large files. -use std::borrow::Cow; use std::collections::BTreeMap; use std::fmt; use std::sync::Arc; @@ -25,6 +24,7 @@ use relay_system::{ SimpleService, }; use serde::Deserialize; +use serde::Serialize; use tokio::io::BufReader; use tokio::sync::oneshot; use tokio::sync::oneshot::error::RecvError; @@ -425,16 +425,12 @@ impl Location { length, other, } = self; - let mut fields = other - .0 - .iter() - .map(|(k, v)| (k.as_str(), Cow::Borrowed(v))) - .collect::>(); - if let Some(length) = length.value() { - fields.push(("upload_length", Cow::Owned(length.to_string()))); - } + let params = LocationQueryParams { + upload_length: length.value(), + other, + }; - let query = serde_urlencoded::to_string(fields)?; + let query = serde_urlencoded::to_string(params)?; match query.as_str() { "" => Ok(format!("/api/{project_id}/upload/{key}/")), _ => Ok(format!("/api/{project_id}/upload/{key}/?{query}")), @@ -473,7 +469,7 @@ pub struct LocationPath { /// Query parameters for the upload endpoint. #[derive(Debug, Deserialize)] #[serde(bound = "L: UploadLength")] -pub struct LocationQueryParams { +pub struct SignedLocationQueryParams { #[serde(alias = "length")] pub upload_length: L, #[serde(alias = "signature")] @@ -482,7 +478,14 @@ pub struct LocationQueryParams { pub other: UploadParams, } -#[derive(Debug, Default)] +#[derive(Debug, Serialize)] +struct LocationQueryParams<'a> { + pub upload_length: Option, + #[serde(flatten)] + pub other: &'a UploadParams, +} + +#[derive(Debug, Default, Serialize)] pub struct UploadParams(BTreeMap); impl<'de> Deserialize<'de> for UploadParams { @@ -589,7 +592,7 @@ impl SignedLocation { impl SignedLocation where L: UploadLength, - LocationQueryParams: for<'de> Deserialize<'de>, + SignedLocationQueryParams: for<'de> Deserialize<'de>, { fn try_from_response(response: Response) -> Result { match response.0.error_for_status() { @@ -624,7 +627,7 @@ where }; // Parse query parameters. - let LocationQueryParams { + let SignedLocationQueryParams { upload_length, upload_signature, other, @@ -831,10 +834,10 @@ mod tests { let url = "signature=foo"; // Can only parse provisional: - let provisional: LocationQueryParams = + let provisional: SignedLocationQueryParams = serde_urlencoded::from_str(url).unwrap(); assert!(provisional.upload_length.0.is_none()); - assert!(serde_urlencoded::from_str::>(url).is_err()); + assert!(serde_urlencoded::from_str::>(url).is_err()); } #[test] @@ -842,10 +845,10 @@ mod tests { let json = r#"signature=foo&length=123"#; // Can only parse provisional: - let provisional: LocationQueryParams = + let provisional: SignedLocationQueryParams = serde_urlencoded::from_str(json).unwrap(); assert_eq!(provisional.upload_length.0, Some(123)); - let full: LocationQueryParams = serde_urlencoded::from_str(json).unwrap(); + let full: SignedLocationQueryParams = serde_urlencoded::from_str(json).unwrap(); assert_eq!(full.upload_length.0, 123); } } From 5f03c3a533bd342bc25981b1d5398ff5698925d6 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Fri, 12 Jun 2026 08:56:48 +0200 Subject: [PATCH 06/11] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 624a49b978b..abb351b2976 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ **Internal**: - Restore top-level _performance_issues_spans. ([#6045](https://github.com/getsentry/relay/pull/6045)) +- Prefix upload location query params for forward compatibility. ([#6076](https://github.com/getsentry/relay/pull/6076)) ## 26.5.2 From 71c3c1177eaefa1cb07452b56374007542451ee8 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Fri, 12 Jun 2026 09:06:23 +0200 Subject: [PATCH 07/11] add test --- relay-server/src/services/upload.rs | 40 ++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index fe0d1ced1ef..cf5fcdfd719 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -563,6 +563,7 @@ impl SignedLocation { location, signature, } = self; + let mut uri = location.try_to_uri()?; uri.push(if uri.ends_with('/') { '?' } else { '&' }); uri.push_str("upload_signature="); @@ -844,11 +845,48 @@ mod tests { fn parse_location_complete() { let json = r#"signature=foo&length=123"#; - // Can only parse provisional: let provisional: SignedLocationQueryParams = serde_urlencoded::from_str(json).unwrap(); assert_eq!(provisional.upload_length.0, Some(123)); let full: SignedLocationQueryParams = serde_urlencoded::from_str(json).unwrap(); assert_eq!(full.upload_length.0, 123); } + + #[test] + fn parse_location_with_other() { + let json = + r#"upload_signature=foo&upload_length=123¬_an_upload_param=123&upload_type=bar"#; + + let provisional: SignedLocationQueryParams = + serde_urlencoded::from_str(json).unwrap(); + insta::assert_debug_snapshot!(provisional, @r#" + SignedLocationQueryParams { + upload_length: Provisional( + Some( + 123, + ), + ), + upload_signature: "foo", + other: UploadParams( + { + "upload_type": "bar", + }, + ), + } + "#); + let full: SignedLocationQueryParams = serde_urlencoded::from_str(json).unwrap(); + insta::assert_debug_snapshot!(full, @r#" + SignedLocationQueryParams { + upload_length: Final( + 123, + ), + upload_signature: "foo", + other: UploadParams( + { + "upload_type": "bar", + }, + ), + } + "#); + } } From ca6f1f7f875d15bb4a94912d8359c2cf2ab5761d Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Fri, 12 Jun 2026 09:33:29 +0200 Subject: [PATCH 08/11] Add legacy verification --- relay-server/src/endpoints/upload.rs | 6 +- relay-server/src/services/upload.rs | 94 ++++++++++++++++++++++------ 2 files changed, 77 insertions(+), 23 deletions(-) diff --git a/relay-server/src/endpoints/upload.rs b/relay-server/src/endpoints/upload.rs index bbd2b80e17b..22330d53c1f 100644 --- a/relay-server/src/endpoints/upload.rs +++ b/relay-server/src/endpoints/upload.rs @@ -31,7 +31,7 @@ use crate::service::ServiceState; use crate::services::objectstore; use crate::services::projects::cache::Project; use crate::services::upload::{ - self, ByteStream, Final, Provisional, SignedLocation, SignedLocationQueryParams, UploadLength, + self, ByteStream, Final, LocationQueryParams, Provisional, SignedLocation, UploadLength, }; use crate::services::upstream::UpstreamRequestError; use crate::statsd::RelayCounters; @@ -201,11 +201,11 @@ async fn handle_patch( meta: RequestMeta, headers: HeaderMap, Path(upload::LocationPath { project_id, key }): Path, - Query(SignedLocationQueryParams { + Query(LocationQueryParams { upload_length, upload_signature, other, - }): Query>, + }): Query>, body: Body, ) -> axum::response::Result { check_kill_switch(&state)?; diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index cf5fcdfd719..8b404a86775 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -425,11 +425,38 @@ impl Location { length, other, } = self; - let params = LocationQueryParams { + #[derive(Debug, Serialize)] + struct QueryParams<'a> { + pub upload_length: Option, + #[serde(flatten)] + pub other: &'a UploadParams, + } + let params = QueryParams { upload_length: length.value(), other, }; + let query = serde_urlencoded::to_string(params)?; + match query.as_str() { + "" => Ok(format!("/api/{project_id}/upload/{key}/")), + _ => Ok(format!("/api/{project_id}/upload/{key}/?{query}")), + } + } + /// Temporary function used to verify legacy signatures. + fn try_to_legacy_uri(&self) -> Result { + let Location { + project_id, + key, + length, + other: _, + } = self; + #[derive(Debug, Serialize)] + struct QueryParams { + pub length: Option, + } + let params = QueryParams { + length: length.value(), + }; let query = serde_urlencoded::to_string(params)?; match query.as_str() { "" => Ok(format!("/api/{project_id}/upload/{key}/")), @@ -469,7 +496,7 @@ pub struct LocationPath { /// Query parameters for the upload endpoint. #[derive(Debug, Deserialize)] #[serde(bound = "L: UploadLength")] -pub struct SignedLocationQueryParams { +pub struct LocationQueryParams { #[serde(alias = "length")] pub upload_length: L, #[serde(alias = "signature")] @@ -478,13 +505,6 @@ pub struct SignedLocationQueryParams { pub other: UploadParams, } -#[derive(Debug, Serialize)] -struct LocationQueryParams<'a> { - pub upload_length: Option, - #[serde(flatten)] - pub other: &'a UploadParams, -} - #[derive(Debug, Default, Serialize)] pub struct UploadParams(BTreeMap); @@ -577,12 +597,24 @@ impl SignedLocation { #[cfg(feature = "processing")] pub fn verify(self, received: DateTime, config: &Config) -> Result, Error> { let public_key = config.public_key().ok_or(Error::SigningFailed)?; - let is_valid = self.signature.verify( + + let mut is_valid = self.signature.verify( self.location.try_to_uri()?.as_bytes(), public_key, received, chrono::Duration::seconds(config.upload().max_age), ); + + // NOTE: This code path can be removed once all legacy URIs are invalid (~1h after rollout) + if !is_valid { + is_valid = self.signature.verify( + self.location.try_to_legacy_uri()?.as_bytes(), + public_key, + received, + chrono::Duration::seconds(config.upload().max_age), + ) + } + match is_valid { true => Ok(self.location), false => Err(Error::InvalidSignature), @@ -593,7 +625,7 @@ impl SignedLocation { impl SignedLocation where L: UploadLength, - SignedLocationQueryParams: for<'de> Deserialize<'de>, + LocationQueryParams: for<'de> Deserialize<'de>, { fn try_from_response(response: Response) -> Result { match response.0.error_for_status() { @@ -628,7 +660,7 @@ where }; // Parse query parameters. - let SignedLocationQueryParams { + let LocationQueryParams { upload_length, upload_signature, other, @@ -835,20 +867,20 @@ mod tests { let url = "signature=foo"; // Can only parse provisional: - let provisional: SignedLocationQueryParams = + let provisional: LocationQueryParams = serde_urlencoded::from_str(url).unwrap(); assert!(provisional.upload_length.0.is_none()); - assert!(serde_urlencoded::from_str::>(url).is_err()); + assert!(serde_urlencoded::from_str::>(url).is_err()); } #[test] fn parse_location_complete() { let json = r#"signature=foo&length=123"#; - let provisional: SignedLocationQueryParams = + let provisional: LocationQueryParams = serde_urlencoded::from_str(json).unwrap(); assert_eq!(provisional.upload_length.0, Some(123)); - let full: SignedLocationQueryParams = serde_urlencoded::from_str(json).unwrap(); + let full: LocationQueryParams = serde_urlencoded::from_str(json).unwrap(); assert_eq!(full.upload_length.0, 123); } @@ -857,10 +889,10 @@ mod tests { let json = r#"upload_signature=foo&upload_length=123¬_an_upload_param=123&upload_type=bar"#; - let provisional: SignedLocationQueryParams = + let provisional: LocationQueryParams = serde_urlencoded::from_str(json).unwrap(); insta::assert_debug_snapshot!(provisional, @r#" - SignedLocationQueryParams { + LocationQueryParams { upload_length: Provisional( Some( 123, @@ -874,9 +906,9 @@ mod tests { ), } "#); - let full: SignedLocationQueryParams = serde_urlencoded::from_str(json).unwrap(); + let full: LocationQueryParams = serde_urlencoded::from_str(json).unwrap(); insta::assert_debug_snapshot!(full, @r#" - SignedLocationQueryParams { + LocationQueryParams { upload_length: Final( 123, ), @@ -889,4 +921,26 @@ mod tests { } "#); } + + #[cfg(feature = "processing")] + #[test] + fn verify_rejects_location_signed_with_legacy_length_param() { + let mut config = Config::default(); + config.regenerate_credentials(false).unwrap(); + + let legacy_uri = "/api/42/upload/my_objectstore_key/?length=123"; + let signature = config.credentials().unwrap().secret_key.sign_with_header( + legacy_uri.as_bytes(), + &SignatureHeader { + timestamp: Some(Utc::now()), + signature_algorithm: None, + }, + ); + let signed_location = SignedLocation::::try_from_str(&format!( + "{legacy_uri}&upload_signature={signature}" + )) + .unwrap(); + + assert!(signed_location.verify(Utc::now(), &config).is_ok()); + } } From 28ecc2723a7571062547de7636ce73bbb1258a58 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Fri, 12 Jun 2026 09:45:12 +0200 Subject: [PATCH 09/11] bump sentry-conventions --- relay-conventions/sentry-conventions | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-conventions/sentry-conventions b/relay-conventions/sentry-conventions index 2a0202257e8..0d3e4fc70af 160000 --- a/relay-conventions/sentry-conventions +++ b/relay-conventions/sentry-conventions @@ -1 +1 @@ -Subproject commit 2a0202257e8baf1b302935aad73be10fa565f8b8 +Subproject commit 0d3e4fc70afe2d7e655617d69825c3a37250c523 From 0d46fb1e56a0799cf579b4c0ba1e2b406ba3a6ea Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Fri, 12 Jun 2026 09:47:57 +0200 Subject: [PATCH 10/11] Apply suggestion from @loewenheim Co-authored-by: Sebastian Zivota --- relay-server/src/services/upload.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index 8b404a86775..642a0961d56 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -23,8 +23,7 @@ use relay_system::{ Addr, AsyncResponse, ConcurrentService, FromMessage, Interface, LoadShed, SendError, Sender, SimpleService, }; -use serde::Deserialize; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use tokio::io::BufReader; use tokio::sync::oneshot; use tokio::sync::oneshot::error::RecvError; From f0566cbdc5221c0e632eb3fc1e5b1a117fed92c3 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Fri, 12 Jun 2026 10:32:22 +0200 Subject: [PATCH 11/11] lint --- relay-server/src/services/upload.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/relay-server/src/services/upload.rs b/relay-server/src/services/upload.rs index 642a0961d56..d915c151482 100644 --- a/relay-server/src/services/upload.rs +++ b/relay-server/src/services/upload.rs @@ -442,6 +442,7 @@ impl Location { } /// Temporary function used to verify legacy signatures. + #[cfg(feature = "processing")] fn try_to_legacy_uri(&self) -> Result { let Location { project_id,