diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 36b39ea19bf..6d47071c7e3 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -10,6 +10,10 @@ `GetTracer` from leaking memory. ([#4906](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4906)) +* Fix `TraceContextPropagator` by validating the first digit of the hex-encoded + `trace-flags` field of the `traceparent` header. + ([#4893](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4893)) + ## 1.6.0 Released 2023-Sep-05 diff --git a/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs b/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs index 4fe9c5503bc..16c2456ad4a 100644 --- a/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs +++ b/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs @@ -195,11 +195,12 @@ internal static bool TryExtractTraceparent(string traceparent, out ActivityTrace return false; } - byte options1; + byte optionsLowByte; try { spanId = ActivitySpanId.CreateFromString(traceparent.AsSpan().Slice(VersionAndTraceIdLength, SpanIdLength)); - options1 = HexCharToByte(traceparent[VersionAndTraceIdAndSpanIdLength + 1]); + _ = HexCharToByte(traceparent[VersionAndTraceIdAndSpanIdLength]); // to verify if there is no bad chars on options position + optionsLowByte = HexCharToByte(traceparent[VersionAndTraceIdAndSpanIdLength + 1]); } catch (ArgumentOutOfRangeException) { @@ -207,7 +208,7 @@ internal static bool TryExtractTraceparent(string traceparent, out ActivityTrace return false; } - if ((options1 & 1) == 1) + if ((optionsLowByte & 1) == 1) { traceOptions |= ActivityTraceFlags.Recorded; } diff --git a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs index a40075bb1e5..58ec321d600 100644 --- a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs +++ b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs @@ -35,7 +35,7 @@ public class W3CTraceContextTests : IDisposable */ private const string W3cTraceContextEnvVarName = "OTEL_W3CTRACECONTEXT"; private static readonly Version AspNetCoreHostingVersion = typeof(Microsoft.AspNetCore.Hosting.Builder.IApplicationBuilderFactory).Assembly.GetName().Version; - private readonly HttpClient httpClient = new HttpClient(); + private readonly HttpClient httpClient = new(); private readonly ITestOutputHelper output; public W3CTraceContextTests(ITestOutputHelper output) @@ -107,7 +107,7 @@ public void W3CTraceContextTestSuiteAsync(string value) } else { - Assert.StartsWith("FAILED (failures=3)", lastLine); + Assert.StartsWith("FAILED (failures=2)", lastLine); } } diff --git a/test/OpenTelemetry.Tests/Trace/Propagation/TraceContextPropagatorTest.cs b/test/OpenTelemetry.Tests/Trace/Propagation/TraceContextPropagatorTest.cs index e932f154ad0..f7dae35547f 100644 --- a/test/OpenTelemetry.Tests/Trace/Propagation/TraceContextPropagatorTest.cs +++ b/test/OpenTelemetry.Tests/Trace/Propagation/TraceContextPropagatorTest.cs @@ -103,12 +103,16 @@ public void IsBlankIfNoHeader() Assert.False(ctx.ActivityContext.IsValid()); } - [Fact] - public void IsBlankIfInvalid() + [Theory] + [InlineData($"00-xyz7651916cd43dd8448eb211c80319c-{SpanId}-01")] + [InlineData($"00-{TraceId}-xyz7c989f97918e1-01")] + [InlineData($"00-{TraceId}-{SpanId}-x1")] + [InlineData($"00-{TraceId}-{SpanId}-1x")] + public void IsBlankIfInvalid(string invalidTraceParent) { var headers = new Dictionary { - { TraceParent, $"00-xyz7651916cd43dd8448eb211c80319c-{SpanId}-01" }, + { TraceParent, invalidTraceParent }, }; var f = new TraceContextPropagator(); @@ -191,8 +195,8 @@ public void DuplicateKeys() // test_tracestate_duplicated_keys Assert.Empty(CallTraceContextPropagator("foo=1,foo=1")); Assert.Empty(CallTraceContextPropagator("foo=1,foo=2")); - Assert.Empty(CallTraceContextPropagator(new string[] { "foo=1", "foo=1" })); - Assert.Empty(CallTraceContextPropagator(new string[] { "foo=1", "foo=2" })); + Assert.Empty(CallTraceContextPropagator(new[] { "foo=1", "foo=1" })); + Assert.Empty(CallTraceContextPropagator(new[] { "foo=1", "foo=2" })); } [Fact] @@ -318,7 +322,7 @@ private static string CallTraceContextPropagator(string[] tracestate) { var headers = new Dictionary { - { TraceParent, new string[] { $"00-{TraceId}-{SpanId}-01" } }, + { TraceParent, new[] { $"00-{TraceId}-{SpanId}-01" } }, { TraceState, tracestate }, }; var f = new TraceContextPropagator();