From cf29bfbad2aa1e2df804f7dce8f7d27f8ee80f55 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= <pkiekowicz@splunk.com>
Date: Wed, 27 Sep 2023 09:55:26 +0200
Subject: [PATCH] Fix `TraceContextPropagator` behavior when trace parent flags
 contains illegal characters. fixes:
 test_traceparent_trace_flags_illegal_characters

---
 global.json                                      |  2 +-
 src/OpenTelemetry.Api/CHANGELOG.md               |  4 ++++
 .../Propagation/TraceContextPropagator.cs        |  7 ++++---
 .../W3CTraceContextTests.cs                      |  4 ++--
 .../Propagation/TraceContextPropagatorTest.cs    | 16 ++++++++++------
 5 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/global.json b/global.json
index 27af8d6b9fb..8c046a29fec 100644
--- 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
index 0345f63aa1c..540c4653047 100644
--- 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
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<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();