diff --git a/opentelemetry/CHANGELOG.md b/opentelemetry/CHANGELOG.md index dbe051e88a..00ab0be983 100644 --- a/opentelemetry/CHANGELOG.md +++ b/opentelemetry/CHANGELOG.md @@ -13,6 +13,7 @@ - Align `Baggage.remove()` signature with `.get()` to take the key as a reference - `Baggage` can't be retrieved from the `Context` directly anymore and needs to be accessed via `context.baggage()` - `with_baggage()` and `current_with_baggage()` override any existing `Baggage` in the `Context` + - `Baggage` keys can't be empty and only allow ASCII visual chars, except `"(),/:;<=>?@[\]{}` (see [RFC7230, Section 3.2.6](https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6)) - Changed `Context` to use a stack to properly handle out of order dropping of `ContextGuard`. This imposes a limit of `65535` nested contexts on a single thread. See #[2378](https://github.com/open-telemetry/opentelemetry-rust/pull/2284) and #[1887](https://github.com/open-telemetry/opentelemetry-rust/issues/1887). - Added additional `name: Option<&str>` parameter to the `event_enabled` method on the `Logger` trait. This allows implementations (SDK, processor, exporters) diff --git a/opentelemetry/benches/baggage.rs b/opentelemetry/benches/baggage.rs index 1854fcd2e3..706353ba5f 100644 --- a/opentelemetry/benches/baggage.rs +++ b/opentelemetry/benches/baggage.rs @@ -11,10 +11,10 @@ const MAX_KEY_VALUE_PAIRS: usize = 64; // Apple M4 Pro // Total Number of Cores: 14 (10 performance and 4 efficiency) // Results: -// set_baggage_static_key_value 12 ns -// set_baggage_static_key 28 ns -// set_baggage_dynamic 60 ns -// set_baggage_dynamic_with_metadata 112 ns +// set_baggage_static_key_value 14 ns +// set_baggage_static_key 26 ns +// set_baggage_dynamic 54 ns +// set_baggage_dynamic_with_metadata 75 ns fn criterion_benchmark(c: &mut Criterion) { set_baggage_static_key_value(c); diff --git a/opentelemetry/src/baggage.rs b/opentelemetry/src/baggage.rs index 69447f5c36..43d796bfd6 100644 --- a/opentelemetry/src/baggage.rs +++ b/opentelemetry/src/baggage.rs @@ -29,6 +29,12 @@ static DEFAULT_BAGGAGE: OnceLock = OnceLock::new(); const MAX_KEY_VALUE_PAIRS: usize = 64; const MAX_LEN_OF_ALL_PAIRS: usize = 8192; +// https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6 +const INVALID_ASCII_KEY_CHARS: [u8; 17] = [ + b'(', b')', b',', b'/', b':', b';', b'<', b'=', b'>', b'?', b'@', b'[', b'\\', b']', b'{', + b'}', b'"', +]; + /// Returns the default baggage, ensuring it is initialized only once. #[inline] fn get_default_baggage() -> &'static Baggage { @@ -160,16 +166,14 @@ impl Baggage { S: Into, { let (key, value, metadata) = (key.into(), value.into(), metadata.into()); - if !key.as_str().is_ascii() { - return None; - } - let entry_content_len = - key_value_metadata_bytes_size(key.as_str(), value.as_str(), metadata.as_str()); let entries_count = self.inner.len(); match self.inner.entry(key) { Entry::Occupied(mut occupied_entry) => { + let key_str = occupied_entry.key().as_str(); + let entry_content_len = + key_value_metadata_bytes_size(key_str, value.as_str(), metadata.as_str()); let prev_content_len = key_value_metadata_bytes_size( - occupied_entry.key().as_str(), + key_str, occupied_entry.get().0.as_str(), occupied_entry.get().1.as_str(), ); @@ -181,9 +185,15 @@ impl Baggage { Some(occupied_entry.insert((value, metadata))) } Entry::Vacant(vacant_entry) => { + let key_str = vacant_entry.key().as_str(); + if !Self::is_key_valid(key_str.as_bytes()) { + return None; + } if entries_count == MAX_KEY_VALUE_PAIRS { return None; } + let entry_content_len = + key_value_metadata_bytes_size(key_str, value.as_str(), metadata.as_str()); let new_content_len = self.kv_content_len + entry_content_len; if new_content_len > MAX_LEN_OF_ALL_PAIRS { return None; @@ -215,11 +225,18 @@ impl Baggage { pub fn iter(&self) -> Iter<'_> { self.into_iter() } + + fn is_key_valid(key: &[u8]) -> bool { + !key.is_empty() + && key + .iter() + .all(|b| b.is_ascii_graphic() && !INVALID_ASCII_KEY_CHARS.contains(b)) + } } /// Get the number of bytes for one key-value pair fn key_value_metadata_bytes_size(key: &str, value: &str, metadata: &str) -> usize { - key.bytes().len() + value.bytes().len() + metadata.bytes().len() + key.len() + value.len() + metadata.len() } /// An iterator over the entries of a [`Baggage`]. @@ -619,4 +636,21 @@ mod tests { baggage.remove("foo"); assert!(baggage.is_empty()); } + + #[test] + fn test_insert_invalid_key() { + let mut baggage = Baggage::default(); + + // empty + baggage.insert("", "1"); + assert!(baggage.is_empty()); + + // non-ascii + baggage.insert("Grüße", "1"); + assert!(baggage.is_empty()); + + // invalid ascii chars + baggage.insert("(example)", "1"); + assert!(baggage.is_empty()); + } }