Fix TraceContextPropagator behavior when trace parent flags contain…

…s illegal characters. fixes: test_traceparent_trace_flags_illegal_characters
open-telemetry · Sep 27, 2023 · cf29bfb · cf29bfb
1 parent bc1badf
commit cf29bfb
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 12 deletions.
diff --git a/global.json b/global.json
@@ -1,6 +1,6 @@
 {
   "sdk": {
     "rollForward": "latestFeature",
-    "version": "8.0.100-rc.1.23463.5"
+    "version": "7.0.401"
   }
 }
diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Fix `TraceContextPropagator` behavior when trace parent flags contains
+  illegal characters.
+  ([#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
@@ -195,19 +195,20 @@ 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)
         {
             // it's ok to still parse tracestate
             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
@@ -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
@@ -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<string, string>
         {
-            { 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<string, string[]>
         {
-            { TraceParent, new string[] { $"00-{TraceId}-{SpanId}-01" } },
+            { TraceParent, new[] { $"00-{TraceId}-{SpanId}-01" } },
             { TraceState, tracestate },
         };
         var f = new TraceContextPropagator();