Validate injected header values in DiagnosticsHandler (#117884)

Author: MihaZupan

src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs

@@ -356,12 +356,14 @@ private void InjectHeaders(Activity currentActivity, HttpRequestMessage request)
         {
             _propagator.Inject(currentActivity, request, static (carrier, key, value) =>
             {
-                if (carrier is HttpRequestMessage request &&
-                    key is not null &&
-                    HeaderDescriptor.TryGet(key, out HeaderDescriptor descriptor) &&
-                    !request.Headers.TryGetHeaderValue(descriptor, out _))
+                if (carrier is HttpRequestMessage request && key is not null)
                 {
-                    request.Headers.TryAddWithoutValidation(descriptor, value);
+                    HeaderDescriptor descriptor = request.Headers.GetHeaderDescriptor(key);
+
+                    if (!request.Headers.Contains(descriptor))
+                    {
+                        request.Headers.Add(descriptor, value);
+                    }
                 }
             });
             request.MarkPropagatorStateInjectedByDiagnosticsHandler();

src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs

@@ -1131,7 +1131,7 @@ private static void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreIte
             }
         }
 
-        private HeaderDescriptor GetHeaderDescriptor(string name)
+        internal HeaderDescriptor GetHeaderDescriptor(string name)
         {
             ArgumentException.ThrowIfNullOrEmpty(name);
 

src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs

@@ -1406,6 +1406,27 @@ await GetFactoryForVersion(UseVersion).CreateClientAndServerAsync(
                 });
         }
 
+        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
+        [InlineData("A B", "Foo", typeof(FormatException))] // Invalid header name
+        [InlineData("Content-Length", "42", typeof(InvalidOperationException))] // Invalid header name for the request headers collection
+        [InlineData("Foo", "Bar\nBaz", typeof(FormatException))] // Invalid header value
+        public async Task SendAsync_PropagatorInjectsInvalidHeaders_Throws(string headerName, string headerValue, Type exceptionType)
+        {
+            using Activity parent = new Activity("parent");
+            parent.SetIdFormat(ActivityIdFormat.W3C);
+            parent.Start();
+
+            using var handler = CreateSocketsHttpHandler(allowAllCertificates: true);
+            handler.ActivityHeadersPropagator = new DelegatingPropagator([headerName], (activity, carrier, setter) => setter(carrier, headerName, headerValue));
+
+            using var client = new HttpClient(handler);
+
+            // Url doesn't matter since the request should fail before hitting the network.
+            var request = CreateRequest(HttpMethod.Get, new Uri("https://microsoft.com"), UseVersion, exactVersion: true);
+
+            await Assert.ThrowsAsync(exceptionType, () => client.SendAsync(TestAsync, request));
+        }
+
         public static IEnumerable<object[]> SocketsHttpHandler_ActivityCreation_MemberData()
         {
             foreach (var currentActivitySet in new bool[] {
@@ -1855,5 +1876,20 @@ private static void AssertHeadersAreInjected(HttpRequestData request, Activity p
             var request = CreateRequest(HttpMethod.Get, uri, Version.Parse(useVersion), exactVersion: true);
             return (request, await client.SendAsync(bool.Parse(testAsync), request, cancellationToken));
         }
+
+        private sealed class DelegatingPropagator(string[] fields, Action<Activity?, object?, DistributedContextPropagator.PropagatorSetterCallback?> inject) : DistributedContextPropagator
+        {
+            public override IReadOnlyCollection<string> Fields => fields;
+
+            public override IEnumerable<KeyValuePair<string, string?>>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter) => [];
+
+            public override void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState)
+            {
+                traceId = null;
+                traceState = null;
+            }
+
+            public override void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter) => inject(activity, carrier, setter);
+        }
     }
 }