Fix OtlpExporterOptionsExtensions.GetHeaders incorrectly throwing for comma in header value

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpExportClient.cs

@@ -42,7 +42,7 @@ protected OtlpExportClient(OtlpExporterOptions options, HttpClient httpClient, s
         }
 
         this.Endpoint = new UriBuilder(exporterEndpoint).Uri;
-        this.Headers = options.GetHeaders<Dictionary<string, string>>((d, k, v) => d.Add(k, v));
+        this.Headers = options.GetHeaders();
         this.HttpClient = httpClient;
     }
 

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptionsExtensions.cs

@@ -22,52 +22,86 @@ internal static class OtlpExporterOptionsExtensions
     private const string MetricsHttpServicePath = "v1/metrics";
     private const string LogsHttpServicePath = "v1/logs";
 
-    public static THeaders GetHeaders<THeaders>(this OtlpExporterOptions options, Action<THeaders, string, string> addHeader)
-        where THeaders : new()
+    public static IReadOnlyDictionary<string, string> GetHeaders(this OtlpExporterOptions options)
     {
         var optionHeaders = options.Headers;
-        var headers = new THeaders();
+        var headers = new Dictionary<string, string>();
         if (!string.IsNullOrEmpty(optionHeaders))
         {
             // According to the specification, URL-encoded headers must be supported.
             optionHeaders = Uri.UnescapeDataString(optionHeaders);
             ReadOnlySpan<char> headersSpan = optionHeaders.AsSpan();
 
+            var nextEqualIndex = headersSpan.IndexOf('=');
+
+            if (nextEqualIndex == -1)
+            {
+                throw CreateInvalidHeaderFormatException();
+            }
+
+            // Skip any leading commas.
+            var leadingCommaIndex = headersSpan.Slice(0, nextEqualIndex).LastIndexOf(',');
+            if (leadingCommaIndex != -1)
+            {
+                headersSpan = headersSpan.Slice(leadingCommaIndex + 1);
+                nextEqualIndex -= leadingCommaIndex + 1;
+            }
+
             while (!headersSpan.IsEmpty)
             {
-                int commaIndex = headersSpan.IndexOf(',');
-                ReadOnlySpan<char> pair;
-                if (commaIndex == -1)
+                var key = headersSpan.Slice(0, nextEqualIndex).Trim().ToString();
+
+                // HTTP header field-names can not be empty: https://www.rfc-editor.org/rfc/rfc7230#section-3.2
+                if (key.Length < 1)
+                {
+                    throw CreateInvalidHeaderFormatException();
+                }
+
+                headersSpan = headersSpan.Slice(nextEqualIndex + 1);
+
+                nextEqualIndex = headersSpan.IndexOf('=');
+
+                string value;
+                if (nextEqualIndex == -1)
                 {
-                    pair = headersSpan;
+                    // Everything until the end of the string can be considered the value.
+                    value = headersSpan.TrimEnd(',').Trim().ToString();
                     headersSpan = [];
                 }
                 else
                 {
-                    pair = headersSpan.Slice(0, commaIndex);
-                    headersSpan = headersSpan.Slice(commaIndex + 1);
-                }
+                    // If we have another = we need to backtrack from it
+                    // and try to find the last comma and consider that as the delimiter.
+                    var potentialValue = headersSpan.Slice(0, nextEqualIndex);
+                    var lastComma = potentialValue.LastIndexOf(',');
 
-                int equalIndex = pair.IndexOf('=');
-                if (equalIndex == -1)
-                {
-                    throw new ArgumentException("Headers provided in an invalid format.");
+                    if (lastComma == -1)
+                    {
+                        throw CreateInvalidHeaderFormatException();
+                    }
+
+                    potentialValue = potentialValue.Slice(0, lastComma);
+
+                    value = potentialValue.TrimEnd(',').Trim().ToString();
+                    headersSpan = headersSpan.Slice(lastComma + 1);
+                    nextEqualIndex -= potentialValue.Length + 1;
                 }
 
-                var key = pair.Slice(0, equalIndex).Trim().ToString();
-                var value = pair.Slice(equalIndex + 1).Trim().ToString();
-                addHeader(headers, key, value);
+                headers.Add(key, value);
             }
         }
 
         foreach (var header in OtlpExporterOptions.StandardHeaders)
         {
-            addHeader(headers, header.Key, header.Value);
+            headers.Add(header.Key, header.Value);
         }
 
         return headers;
     }
 
+    private static ArgumentException CreateInvalidHeaderFormatException()
+        => new("Headers provided in an invalid format. Use: header=value,header=value");
+
     public static OtlpExporterTransmissionHandler GetExportTransmissionHandler(this OtlpExporterOptions options, ExperimentalOptions experimentalOptions, OtlpSignalType otlpSignalType)
     {
         var exportClient = GetExportClient(options, otlpSignalType);

test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsExtensionsTests.cs

@@ -24,7 +24,7 @@ public void GetHeaders_NoOptionHeaders_ReturnsStandardHeaders(string? optionHead
             Headers = optionHeaders,
         };
 
-        var headers = options.GetHeaders<Dictionary<string, string>>((d, k, v) => d.Add(k, v));
+        var headers = options.GetHeaders();
 
         Assert.Equal(OtlpExporterOptions.StandardHeaders.Length, headers.Count);
 
@@ -36,8 +36,8 @@ public void GetHeaders_NoOptionHeaders_ReturnsStandardHeaders(string? optionHead
 
     [Theory]
     [InlineData(" ")]
-    [InlineData(",key1=value1,key2=value2,")]
-    [InlineData(",,key1=value1,,key2=value2,,")]
+    [InlineData(",")]
+    [InlineData("=value1")]
     [InlineData("key1")]
     public void GetHeaders_InvalidOptionHeaders_ThrowsArgumentException(string inputOptionHeaders)
     {
@@ -49,12 +49,17 @@ public void GetHeaders_InvalidOptionHeaders_ThrowsArgumentException(string input
     [InlineData("key1=value1", "key1=value1")]
     [InlineData("key1=value1,key2=value2", "key1=value1,key2=value2")]
     [InlineData("key1=value1,key2=value2,key3=value3", "key1=value1,key2=value2,key3=value3")]
+    [InlineData("key1=value1,value2", "key1=value1,value2")]
+    [InlineData("key1=value1,value2,key2=value3", "key1=value1,value2,key2=value3")]
     [InlineData(" key1 = value1 , key2=value2 ", "key1=value1,key2=value2")]
     [InlineData("key1= value with spaces ,key2=another value", "key1=value with spaces,key2=another value")]
-    [InlineData("=value1", "=value1")]
     [InlineData("key1=", "key1=")]
     [InlineData("key1=value1%2Ckey2=value2", "key1=value1,key2=value2")]
     [InlineData("key1=value1%2Ckey2=value2%2Ckey3=value3", "key1=value1,key2=value2,key3=value3")]
+    [InlineData("key1=value1%2Cvalue2", "key1=value1,value2")]
+    [InlineData("key1=value1%2Cvalue2%2Ckey2=value3", "key1=value1,value2,key2=value3")]
+    [InlineData(",key1=value1,key2=value2,", "key1=value1,key2=value2")]
+    [InlineData(",,key1=value1,,key2=value2,,", "key1=value1,key2=value2")]
     public void GetHeaders_ValidAndUrlEncodedHeaders_ReturnsCorrectHeaders(string inputOptionHeaders, string expectedNormalizedOptional)
     {
         VerifyHeaders(inputOptionHeaders, expectedNormalizedOptional);
@@ -169,33 +174,21 @@ private static void VerifyHeaders(string inputOptionHeaders, string expectedNorm
 
         if (expectException)
         {
-            Assert.Throws<ArgumentException>(() =>
-                options.GetHeaders<Dictionary<string, string>>((d, k, v) => d.Add(k, v)));
+            Assert.Throws<ArgumentException>(() => options.GetHeaders());
             return;
         }
 
-        var headers = options.GetHeaders<Dictionary<string, string>>((d, k, v) => d.Add(k, v));
-        var expectedOptional = new Dictionary<string, string>();
+        var headers = options.GetHeaders();
 
-        if (!string.IsNullOrEmpty(expectedNormalizedOptional))
-        {
-            foreach (var segment in expectedNormalizedOptional.Split([','], StringSplitOptions.RemoveEmptyEntries))
-            {
-                var parts = segment.Split(['='], 2);
-                expectedOptional.Add(parts[0].Trim(), parts[1].Trim());
-            }
-        }
+        var actual = string.Join(",", headers.Select(h => $"{h.Key}={h.Value}"));
 
-        Assert.Equal(OtlpExporterOptions.StandardHeaders.Length + expectedOptional.Count, headers.Count);
-
-        foreach (var kvp in expectedOptional)
+        var expected = expectedNormalizedOptional;
+        if (expected.Length > 0)
         {
-            Assert.Contains(headers, h => h.Key == kvp.Key && h.Value == kvp.Value);
+            expected += ',';
         }
+        expected += string.Join(",", OtlpExporterOptions.StandardHeaders.Select(h => $"{h.Key}={h.Value}"));
 
-        foreach (var std in OtlpExporterOptions.StandardHeaders)
-        {
-            Assert.Contains(headers, h => h.Key == std.Key && h.Value == std.Value);
-        }
+        Assert.Equal(expected, actual);
     }
 }