Skip to content

Add comprehensive defensive parsing tests for TraceState and Baggage propagators #3040

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions opentelemetry-sdk/src/logs/in_memory_exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ impl InMemoryLogExporterBuilder {

/// If set, the records will not be [`InMemoryLogExporter::reset`] on shutdown.
#[cfg(test)]
#[allow(dead_code)]
pub(crate) fn keep_records_on_shutdown(self) -> Self {
Self {
reset_on_shutdown: false,
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-sdk/src/logs/logger_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,8 @@ mod tests {
tracer.in_span("test-span", |cx| {
let ambient_ctxt = cx.span().span_context().clone();
let explicit_ctxt = TraceContext {
trace_id: TraceId::from_u128(13),
span_id: SpanId::from_u64(14),
trace_id: TraceId::from_bytes([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 13]),
span_id: SpanId::from_bytes([0, 0, 0, 0, 0, 0, 0, 14]),
trace_flags: None,
};

Expand Down
163 changes: 163 additions & 0 deletions opentelemetry-sdk/src/propagation/baggage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,167 @@
}
}
}

#[rustfmt::skip]
fn malformed_baggage_test_data() -> Vec<(String, &'static str)> {
vec![
// Empty and whitespace
("".to_string(), "empty header"),
(" ".to_string(), "whitespace only header"),

// Malformed key-value pairs
("key_without_value".to_string(), "missing equals sign"),
("=value_without_key".to_string(), "missing key"),
("key=".to_string(), "empty value allowed"),
("=".to_string(), "empty key and value"),

// Multiple equals signs
("key=value=extra".to_string(), "multiple equals signs"),
("key=val=ue=more".to_string(), "many equals signs"),

// Control characters and non-printable characters
("key=val\x00ue".to_string(), "null character in value"),
("key\x01=value".to_string(), "control character in key"),
("key=value\x7F".to_string(), "DEL character in value"),
("key\t=value".to_string(), "tab character in key"),
("key=val\nue".to_string(), "newline in value"),
("key=val\rue".to_string(), "carriage return in value"),

// Invalid UTF-8 sequences (these will be handled by percent decoding)
("key=%80".to_string(), "invalid UTF-8 start byte"),
("key=%C2".to_string(), "incomplete UTF-8 sequence"),
("key=%ED%A0%80".to_string(), "UTF-8 surrogate"),

// Very long keys and values
(format!("{}=value", "a".repeat(1000)), "very long key"),
(format!("key={}", "v".repeat(1000)), "very long value"),
(format!("{}={}", "k".repeat(500), "v".repeat(500)), "long key and value"),

// Many entries to test memory usage
((0..1000).map(|i| format!("key{}=val{}", i, i)).collect::<Vec<_>>().join(","), "many entries"),

// Malformed metadata
("key=value;".to_string(), "empty metadata"),
("key=value;;".to_string(), "double semicolon"),
("key=value;meta;".to_string(), "trailing semicolon"),
("key=value;meta=".to_string(), "metadata with empty value"),

// Mixed valid and invalid entries
("valid_key=valid_value,invalid_key,another_valid=value".to_string(), "mixed valid and invalid"),
("key1=val1,=,key2=val2".to_string(), "empty entry in middle"),

// Extreme whitespace
(" key1 = val1 , key2 = val2 ".to_string(), "excessive whitespace"),

// Special characters that might cause issues
("key=value,".to_string(), "trailing comma"),
(",key=value".to_string(), "leading comma"),
("key=value,,".to_string(), "double comma"),
("key=val,ue,key2=val2".to_string(), "comma in entry"),

// Unicode characters
("café=bücher".to_string(), "unicode characters"),
("key=🔥".to_string(), "emoji in value"),
("🗝️=value".to_string(), "emoji in key"),
]
}

#[test]
fn extract_baggage_defensive_parsing() {
let propagator = BaggagePropagator::new();

for (malformed_header, description) in malformed_baggage_test_data() {
let mut extractor: HashMap<String, String> = HashMap::new();
extractor.insert(BAGGAGE_HEADER.to_string(), malformed_header.clone());

// The main requirement is that parsing doesn't crash or hang
let context = propagator.extract(&extractor);
let baggage = context.baggage();

// Baggage should be created without crashing, regardless of content
// Invalid entries should be ignored or handled gracefully
assert!(
baggage.len() <= 1000, // Reasonable upper bound
"Too many baggage entries extracted from malformed header: {} ({})",
description, baggage.len()

Check warning on line 405 in opentelemetry-sdk/src/propagation/baggage.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/propagation/baggage.rs#L404-L405

Added lines #L404 - L405 were not covered by tests
);

// No entry should have an empty key (our validation should prevent this)
for (key, _) in baggage {
assert!(
!key.as_str().is_empty(),
"Empty key found in baggage from header: {} ({})",

Check warning on line 412 in opentelemetry-sdk/src/propagation/baggage.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/propagation/baggage.rs#L412

Added line #L412 was not covered by tests
malformed_header, description
);
}
}
}

#[test]
fn extract_baggage_memory_safety() {
let propagator = BaggagePropagator::new();

// Test extremely long header to ensure no memory exhaustion
let very_long_header = format!("key={}", "x".repeat(100_000));
let mut extractor: HashMap<String, String> = HashMap::new();
extractor.insert(BAGGAGE_HEADER.to_string(), very_long_header);

let context = propagator.extract(&extractor);
let baggage = context.baggage();

// Should handle gracefully without crashing
assert!(baggage.len() <= 1);

// Test header with many small entries
let many_entries: Vec<String> = (0..10_000)
.map(|i| format!("k{}=v{}", i, i))
.collect();
let large_header = many_entries.join(",");

let mut extractor2: HashMap<String, String> = HashMap::new();
extractor2.insert(BAGGAGE_HEADER.to_string(), large_header);

let context2 = propagator.extract(&extractor2);
let baggage2 = context2.baggage();

// Should handle gracefully, possibly truncating or limiting entries
assert!(baggage2.len() <= 10_000);

// Verify no extremely long keys or values made it through
for (key, (value, _)) in baggage2 {
assert!(
key.as_str().len() <= 1000,
"Key too long: {} chars", key.as_str().len()

Check warning on line 453 in opentelemetry-sdk/src/propagation/baggage.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/propagation/baggage.rs#L453

Added line #L453 was not covered by tests
);
assert!(
value.as_str().len() <= 1000,
"Value too long: {} chars", value.as_str().len()

Check warning on line 457 in opentelemetry-sdk/src/propagation/baggage.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/propagation/baggage.rs#L457

Added line #L457 was not covered by tests
);
}
}

#[test]
fn extract_baggage_percent_encoding_edge_cases() {
let propagator = BaggagePropagator::new();

let test_cases = vec![
("%", "lone percent sign"),
("key=%", "percent at end"),
("key=%2", "incomplete percent encoding"),
("key=%ZZ", "invalid hex in percent encoding"),
("key=%2G", "invalid hex digit"),
("key=%%20", "double percent"),
("key=%20%20%20", "multiple encoded spaces"),
];

for (header, _description) in test_cases {
let mut extractor: HashMap<String, String> = HashMap::new();
extractor.insert(BAGGAGE_HEADER.to_string(), header.to_string());

// Should not crash on invalid percent encoding
let context = propagator.extract(&extractor);
let _baggage = context.baggage();
// Test passes if no panic occurs
}
}
}
195 changes: 195 additions & 0 deletions opentelemetry-sdk/src/propagation/trace_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,4 +295,199 @@

assert_eq!(Extractor::get(&injector, TRACESTATE_HEADER), Some(state))
}

#[rustfmt::skip]
fn malformed_traceparent_test_data() -> Vec<(String, &'static str)> {
vec![
// Existing invalid cases are already covered, adding more edge cases
("".to_string(), "completely empty"),
(" ".to_string(), "whitespace only"),
("00".to_string(), "too few parts"),
("00-".to_string(), "incomplete with separator"),
("00--00".to_string(), "missing trace ID"),
("00-4bf92f3577b34da6a3ce929d0e0e4736--01".to_string(), "missing span ID"),
("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-".to_string(), "missing flags"),

// Very long inputs
(format!("00-{}-00f067aa0ba902b7-01", "a".repeat(1000)), "very long trace ID"),
(format!("00-4bf92f3577b34da6a3ce929d0e0e4736-{}-01", "b".repeat(1000)), "very long span ID"),
(format!("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-{}", "c".repeat(1000)), "very long flags"),

// Non-hex characters
("00-4bf92f3577b34da6a3ce929d0e0e473g-00f067aa0ba902b7-01".to_string(), "non-hex in trace ID"),
("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b$-01".to_string(), "non-hex in span ID"),
("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-0g".to_string(), "non-hex in flags"),

// Unicode and special characters
("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01🔥".to_string(), "emoji in flags"),
("00-café4da6a3ce929d0e0e4736-00f067aa0ba902b7-01".to_string(), "unicode in trace ID"),
("00-4bf92f3577b34da6a3ce929d0e0e4736-café67aa0ba902b7-01".to_string(), "unicode in span ID"),

// Control characters (these may be trimmed by the parser)
("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01\x00".to_string(), "null terminator"),

// Multiple separators
("00--4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01".to_string(), "double separator"),
("00-4bf92f3577b34da6a3ce929d0e0e4736--00f067aa0ba902b7-01".to_string(), "double separator middle"),
("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7--01".to_string(), "double separator end"),
]
}

#[rustfmt::skip]
fn malformed_tracestate_header_test_data() -> Vec<(String, &'static str)> {
vec![
// Very long tracestate headers
(format!("key={}", "x".repeat(100_000)), "extremely long value"),
(format!("{}=value", "k".repeat(100_000)), "extremely long key"),
((0..10_000).map(|i| format!("k{}=v{}", i, i)).collect::<Vec<_>>().join(","), "many entries"),

// Malformed but should not crash
("key=value,malformed".to_string(), "mixed valid and invalid"),
("=value1,key2=value2,=value3".to_string(), "multiple empty keys"),
("key1=value1,,key2=value2".to_string(), "empty entry"),
("key1=value1,key2=".to_string(), "empty value"),
("key1=,key2=value2".to_string(), "another empty value"),

// Control characters and special cases
("key=val\x00ue".to_string(), "null character"),
("key=val\nue".to_string(), "newline character"),
("key=val\tue".to_string(), "tab character"),
("key\x01=value".to_string(), "control character in key"),

// Unicode
("café=bücher".to_string(), "unicode key and value"),
("🔥=🎉".to_string(), "emoji key and value"),
("ключ=значение".to_string(), "cyrillic"),

// Invalid percent encoding patterns
("key=%ZZ".to_string(), "invalid hex in percent encoding"),
("key=%".to_string(), "incomplete percent encoding"),
("key=%%".to_string(), "double percent"),
]
}

#[test]
fn extract_w3c_defensive_traceparent() {
let propagator = TraceContextPropagator::new();

// Test all the malformed traceparent cases
for (invalid_header, reason) in malformed_traceparent_test_data() {
let mut extractor = HashMap::new();
extractor.insert(TRACEPARENT_HEADER.to_string(), invalid_header.clone());

// Should not crash and should return empty context
let result = propagator.extract(&extractor);
assert_eq!(
result.span().span_context(),
&SpanContext::empty_context(),
"Failed to reject invalid traceparent: {} ({})", invalid_header, reason

Check warning on line 383 in opentelemetry-sdk/src/propagation/trace_context.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/propagation/trace_context.rs#L383

Added line #L383 was not covered by tests
);
}
}

#[test]
fn extract_w3c_defensive_tracestate() {
let propagator = TraceContextPropagator::new();

// Use a valid traceparent with various malformed tracestate headers
let valid_parent = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01";

for (malformed_state, description) in malformed_tracestate_header_test_data() {
let mut extractor = HashMap::new();
extractor.insert(TRACEPARENT_HEADER.to_string(), valid_parent.to_string());
extractor.insert(TRACESTATE_HEADER.to_string(), malformed_state.clone());

// Should not crash - malformed tracestate should fallback to default
let result = propagator.extract(&extractor);
let span = result.span();
let span_context = span.span_context();

// Should still have valid span context from traceparent
assert!(span_context.is_valid(), "Valid traceparent should create valid context despite malformed tracestate: {}", description);

// Tracestate should either be default or contain only valid entries
let trace_state = span_context.trace_state();
let header = trace_state.header();

// Verify the tracestate header is reasonable (no extremely long result)
assert!(
header.len() <= malformed_state.len() + 1000,
"TraceState header grew unreasonably for input '{}' ({}): {} -> {}",
malformed_state, description, malformed_state.len(), header.len()

Check warning on line 416 in opentelemetry-sdk/src/propagation/trace_context.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/propagation/trace_context.rs#L415-L416

Added lines #L415 - L416 were not covered by tests
);
}
}

#[test]
fn extract_w3c_memory_safety() {
let propagator = TraceContextPropagator::new();

// Test extremely long traceparent
let very_long_traceparent = format!(
"00-{}-{}-01",
"a".repeat(1_000_000), // Very long trace ID
"b".repeat(1_000_000) // Very long span ID
);

let mut extractor = HashMap::new();
extractor.insert(TRACEPARENT_HEADER.to_string(), very_long_traceparent);

// Should not crash or consume excessive memory
let result = propagator.extract(&extractor);
assert_eq!(result.span().span_context(), &SpanContext::empty_context());

// Test with both long traceparent and tracestate
let long_tracestate = format!("key={}", "x".repeat(1_000_000));
let mut extractor2 = HashMap::new();
extractor2.insert(TRACEPARENT_HEADER.to_string(), "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01".to_string());
extractor2.insert(TRACESTATE_HEADER.to_string(), long_tracestate);

// Should handle gracefully without excessive memory usage
let result2 = propagator.extract(&extractor2);
let span2 = result2.span();
let span_context2 = span2.span_context();
assert!(span_context2.is_valid());
}

#[test]
fn extract_w3c_boundary_conditions() {
let propagator = TraceContextPropagator::new();

// Test boundary conditions for version and flags
let boundary_test_cases = vec![
("ff-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", "max version"),
("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-ff", "max flags"),
("00-00000000000000000000000000000001-0000000000000001-01", "minimal valid IDs"),
("00-ffffffffffffffffffffffffffffffff-ffffffffffffffff-01", "maximal valid IDs"),
];

for (test_header, description) in boundary_test_cases {
let mut extractor = HashMap::new();
extractor.insert(TRACEPARENT_HEADER.to_string(), test_header.to_string());

let result = propagator.extract(&extractor);
let span = result.span();
let span_context = span.span_context();

// These should be handled according to W3C spec
// The test passes if no panic occurs and behavior is consistent
match description {
"max version" => {
// Version 255 should be accepted (as per spec, parsers should accept unknown versions)
// But our implementation might reject it - either behavior is defensive
}
"max flags" => {
// Max flags should be accepted but masked to valid bits
if span_context.is_valid() {
// Only the sampled bit should be preserved, so check it's either sampled or not
let is_sampled = span_context.trace_flags().is_sampled();
assert!(is_sampled || !is_sampled); // This always passes, just ensuring no panic

Check warning on line 484 in opentelemetry-sdk/src/propagation/trace_context.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/propagation/trace_context.rs#L483-L484

Added lines #L483 - L484 were not covered by tests
}
}
_ => {
// Other cases should work normally
}
}
}
}
}
Loading
Loading