fix(dynamic-config): use matches_any_origin to scrub HTTP hosts in spans
#3939
Conversation
relay-dynamic-config/src/global.rs
Outdated
| while let Some(value) = seq.next_element()? { | ||
| let this = Host::parse(value); | ||
| if let Ok(t) = this { | ||
| values.push(t) | ||
| } | ||
| } |
There was a problem hiding this comment.
This visitor still fails if either one of the elements in the list has the wrong type or even the entire object is of the wrong type (more likely).
We can keep this fallible and not ignore invalid hosts (it's not user config, they should be always valid) and instead handle the entire error of the visitor at the deserialize.deserialize_* call.
relay-dynamic-config/src/global.rs
Outdated
| } | ||
| } | ||
|
|
||
| deserializer.deserialize_any(HostVisitor) |
There was a problem hiding this comment.
| deserializer.deserialize_any(HostVisitor) | |
| deserializer.deserialize_seq(HostVisitor) |
Can handle the error here.
Good idea, this would even come with wildcard matching built in. Parsing into |
…vec-host-deserializer' into fix/dynamic-config/vec-host-deserializer
relay-event-normalization/src/normalize/span/description/mod.rs
Outdated
Show resolved
Hide resolved
| @@ -29,6 +29,7 @@ relay-log = { workspace = true } | |||
| relay-protocol = { workspace = true } | |||
| relay-statsd = { workspace = true } | |||
| relay-ua = { workspace = true } | |||
| relay-filter = { workspace = true } | |||
There was a problem hiding this comment.
nit: We should move the utility function to relay-common instead of pulling in relay-filter here.
aldy505
changed the title
matches_any_origin to scrub HTTP hosts in spans
|
@jjbayer all greens now |
|
@aldy505 we had to revert this PR because it caused panics for URLs that contain |
…spans (#3939) Closes #3936 ### Legal Boilerplate Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms. --------- Co-authored-by: Joris Bayer <[email protected]>
Re-apply #3939, which caused panics in urls with brackets. This PR contains a minimal fix to prevent the panic: 22233f3 --------- Co-authored-by: Reinaldy Rafli <[email protected]>
Closes #3936
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.