Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support URL-encoded values for OTEL_EXPORTER_OTLP_HEADERS #1578

Merged
merged 12 commits into from
Mar 1, 2024
4 changes: 4 additions & 0 deletions opentelemetry-otlp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## vNext

### Fixed

- URL encoded values in `OTEL_EXPORTER_OTLP_HEADERS` are now correctly decoded. [#1578](https://github.com/open-telemetry/opentelemetry-rust/pull/1578)

### Added

- Added `DeltaTemporalitySelector` ([#1568])
Expand Down
8 changes: 6 additions & 2 deletions opentelemetry-otlp/src/exporter/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,11 @@
pub fn with_headers(mut self, headers: HashMap<String, String>) -> Self {
// headers will be wrapped, so we must do some logic to unwrap first.
let mut inst_headers = self.http_config.headers.unwrap_or_default();
inst_headers.extend(headers);
inst_headers.extend(
headers
.into_iter()
.map(|(key, value)| (key, super::url_decode(&value).unwrap_or(value))),
);

Check warning on line 127 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/http/mod.rs#L123-L127

Added lines #L123 - L127 were not covered by tests
self.http_config.headers = Some(inst_headers);
self
}
Expand Down Expand Up @@ -310,7 +314,7 @@
headers.extend(parse_header_string(input).filter_map(|(key, value)| {
Some((
HeaderName::from_str(key).ok()?,
HeaderValue::from_str(value).ok()?,
HeaderValue::from_str(&value).ok()?,
))
}));
}
Expand Down
75 changes: 70 additions & 5 deletions opentelemetry-otlp/src/exporter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,18 +211,53 @@ impl<B: HasExportConfig> WithExportConfig for B {
}

#[cfg(any(feature = "grpc-tonic", feature = "http-proto"))]
fn parse_header_string(value: &str) -> impl Iterator<Item = (&str, &str)> {
fn url_decode(value: &str) -> Option<String> {
let mut result = String::with_capacity(value.len());
let mut chars_to_decode = Vec::<u8>::new();
let mut all_chars = value.chars();

loop {
let ch = all_chars.next();

if ch.is_some() && ch.unwrap() == '%' {
chars_to_decode.push(
u8::from_str_radix(&format!("{}{}", all_chars.next()?, all_chars.next()?), 16)
.ok()?,
);
continue;
}

if !chars_to_decode.is_empty() {
result.push_str(std::str::from_utf8(&chars_to_decode).ok()?);
chars_to_decode.clear();
}

if let Some(c) = ch {
result.push(c);
} else {
return Some(result);
}
}
}

#[cfg(any(feature = "grpc-tonic", feature = "http-proto"))]
fn parse_header_string(value: &str) -> impl Iterator<Item = (&str, String)> {
value
.split_terminator(',')
.map(str::trim)
.filter_map(parse_header_key_value_string)
}

#[cfg(any(feature = "grpc-tonic", feature = "http-proto"))]
fn parse_header_key_value_string(key_value_string: &str) -> Option<(&str, &str)> {
fn parse_header_key_value_string(key_value_string: &str) -> Option<(&str, String)> {
key_value_string
.split_once('=')
.map(|(key, value)| (key.trim(), value.trim()))
.map(|(key, value)| {
(
key.trim(),
url_decode(value.trim()).unwrap_or(value.to_string()),
)
})
.filter(|(key, value)| !key.is_empty() && !value.is_empty())
}

Expand Down Expand Up @@ -267,6 +302,24 @@ mod tests {
);
}

#[test]
fn test_url_decode() {
let test_cases = vec![
// Format: (encoded, expected_decoded)
("v%201", Some("v 1")),
("v 1", Some("v 1")),
("%C3%B6%C3%A0%C2%A7%C3%96abcd%C3%84", Some("öà§ÖabcdÄ")),
("v%XX1", None),
];

for (encoded, expected_decoded) in test_cases {
assert_eq!(
super::url_decode(encoded),
expected_decoded.map(|v| v.to_string()),
)
}
}

#[test]
fn test_parse_header_string() {
let test_cases = vec![
Expand All @@ -280,7 +333,10 @@ mod tests {
for (input_str, expected_headers) in test_cases {
assert_eq!(
super::parse_header_string(input_str).collect::<Vec<_>>(),
expected_headers,
expected_headers
.into_iter()
.map(|(k, v)| (k, v.to_string()))
.collect::<Vec<_>>(),
)
}
}
Expand All @@ -290,6 +346,15 @@ mod tests {
let test_cases = vec![
// Format: (input_str, expected_header)
("k1=v1", Some(("k1", "v1"))),
(
"Authentication=Basic AAA",
Some(("Authentication", "Basic AAA")),
),
(
"Authentication=Basic%20AAA",
Some(("Authentication", "Basic AAA")),
),
("k1=%XX", Some(("k1", "%XX"))),
("", None),
("=v1", None),
("k1=", None),
Expand All @@ -298,7 +363,7 @@ mod tests {
for (input_str, expected_headers) in test_cases {
assert_eq!(
super::parse_header_key_value_string(input_str),
expected_headers,
expected_headers.map(|(k, v)| (k, v.to_string())),
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/src/exporter/tonic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ fn parse_headers_from_env(signal_headers_var: &str) -> HeaderMap {
.filter_map(|(key, value)| {
Some((
HeaderName::from_str(key).ok()?,
HeaderValue::from_str(value).ok()?,
HeaderValue::from_str(&value).ok()?,
))
})
.collect::<HeaderMap>()
Expand Down
Loading