[API] Baggage - Spec alignment changes

src/OpenTelemetry.Api/CHANGELOG.md

@@ -10,6 +10,10 @@ Notes](../../RELEASENOTES.md).
   Add support for using environment variables as context propagation carriers.
   ([#7174](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7174))
 
+* Fix `BaggagePropagator` to correctly follow Key and Value Encoding rules as mentioned
+  the [W3C Baggage specification](https://www.w3.org/TR/baggage/#key-and-value-encoding).
+  [#7051](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7051)
+
 * Update `TraceContextPropagator` to support the W3C randomness flag.
   ([#7301](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7301))
 

src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs

@@ -2,12 +2,9 @@
 // SPDX-License-Identifier: Apache-2.0
 
 #if NET
-#if NET9_0_OR_GREATER
 using System.Buffers;
-#endif
 using System.Diagnostics.CodeAnalysis;
 #endif
-using System.Net;
 using System.Text;
 using OpenTelemetry.Internal;
 
@@ -19,12 +16,44 @@ namespace OpenTelemetry.Context.Propagation;
 public class BaggagePropagator : TextMapPropagator
 {
     internal const string BaggageHeaderName = "baggage";
+    private const string Hex = "0123456789ABCDEF";
 
     private const int MaxBaggageLength = 8192;
     private const int MaxBaggageItems = 180;
 
-#if NET9_0_OR_GREATER
-    private static readonly SearchValues<char> DecodeHints = SearchValues.Create('%', '+');
+#if NET
+    private static readonly SearchValues<char> DecodeHints = SearchValues.Create("%");
+
+    private static readonly SearchValues<char> ValidKeySearcher = SearchValues.Create(
+        "!#$%&'*+-.^_`|~0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz");
+
+    // W3C Baggage 3.3 baggage-octet, '%' excluded so raw '%' is always encoded as %25
+    private static readonly SearchValues<char> ValidValueSearcher = SearchValues.Create(
+        "!#$&'()*+-./:<=>?@[]^_`|~0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz{}");
+
+#else
+
+    private static readonly char[] ValidKeyChars =
+    [
+        '!', '#', '$', '%', '&', '\'', '*', '+', '-', '.', '^', '_', '`', '|', '~',
+        '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+        'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M',
+        'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z',
+        'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
+        'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z',
+    ];
+
+    // baggage-octet minus %, so raw % is always encoded as %25
+    private static readonly char[] ValidValueChars =
+    [
+        '!', '#', '$', '&', '\'', '(', ')', '*', '+', '-', '.', '/', ':',
+        '<', '=', '>', '?', '@', '[', ']', '^', '_', '`', '{', '|', '}', '~',
+        '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+        'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M',
+        'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z',
+        'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
+        'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z',
+    ];
 #endif
 
     /// <inheritdoc/>
@@ -108,8 +137,13 @@ public override void Inject<T>(PropagationContext context, T carrier, Action<T,
                     continue;
                 }
 
-                var encodedKey = WebUtility.UrlEncode(item.Key);
-                var encodedValue = WebUtility.UrlEncode(item.Value);
+                if (!IsValidKey(item.Key))
+                {
+                    continue;
+                }
+
+                var encodedKey = item.Key;
+                var encodedValue = EncodeValue(item.Value);
                 var baggageItemLength = encodedKey.Length + encodedValue.Length + 1;
 
                 if (baggage.Length > 0)
@@ -167,7 +201,7 @@ internal static bool TryExtractBaggage(
             while (!remaining.IsEmpty)
             {
                 var pair = ReadNextSegment(ref remaining, ',');
-                baggageLength += pair.Length + 1; // pair and comma
+                baggageLength += pair.Length + 1;
 
                 if (baggageLength >= MaxBaggageLength || baggageDictionary?.Count >= MaxBaggageItems)
                 {
@@ -183,6 +217,13 @@ internal static bool TryExtractBaggage(
 
                 var rawKey = pair.Slice(0, separatorIndex).Trim();
 
+                if (!IsValidKey(rawKey))
+                {
+                    continue;
+                }
+
+                var key = rawKey.ToString();
+
                 var rawValue = pair.Slice(separatorIndex + 1);
 
                 var semicolonIndex = rawValue.IndexOf(';');
@@ -193,7 +234,6 @@ internal static bool TryExtractBaggage(
 
                 rawValue = rawValue.Trim();
 
-                var key = DecodeIfNeeded(rawKey);
                 var value = DecodeIfNeeded(rawValue);
 
                 if (string.IsNullOrEmpty(key) || string.IsNullOrEmpty(value))
@@ -225,10 +265,212 @@ private static ReadOnlySpan<char> ReadNextSegment(ref ReadOnlySpan<char> remaini
         return result;
     }
 
-    private static string DecodeIfNeeded(ReadOnlySpan<char> value) =>
-#if NET9_0_OR_GREATER
-        value.ContainsAny(DecodeHints) ? WebUtility.UrlDecode(value.ToString()) : value.ToString();
+    private static string EncodeKey(ReadOnlySpan<char> key) => Encode(key, isKey: true);
+
+    private static string EncodeValue(ReadOnlySpan<char> value) => Encode(value, isKey: false);
+
+    private static string Encode(ReadOnlySpan<char> value, bool isKey)
+    {
+#if NET
+        if (!value.ContainsAnyExcept(isKey ? ValidKeySearcher : ValidValueSearcher))
+        {
+            return value.ToString();
+        }
+#else
+        var validChars = isKey ? ValidKeyChars : ValidValueChars;
+        var allValid = true;
+        foreach (var c in value)
+        {
+            if (Array.IndexOf(validChars, c) < 0)
+            {
+                allValid = false;
+                break;
+            }
+        }
+
+        if (allValid)
+        {
+            return value.ToString();
+        }
+#endif
+
+        var sb = new StringBuilder(value.Length);
+
+#if NET
+        Span<byte> utf8Buffer = stackalloc byte[4];
+        foreach (var rune in value.EnumerateRunes())
+        {
+            if (rune.IsAscii)
+            {
+                var c = (char)rune.Value;
+                if (isKey ? !IsValidKey(c) : !IsValidValueChar(c))
+                {
+                    AppendPercentEncoded(sb, (byte)c);
+                }
+                else
+                {
+                    sb.Append(c);
+                }
+            }
+            else
+            {
+                // Non-ASCII rune: always encode as UTF-8 bytes.
+                // This correctly handles non-BMP scalar values (emoji, etc.)
+                // because Rune represents the full codepoint, not a surrogate half.
+                var byteCount = rune.EncodeToUtf8(utf8Buffer);
+                foreach (var b in utf8Buffer[..byteCount])
+                {
+                    AppendPercentEncoded(sb, b);
+                }
+            }
+        }
+#else
+        var i = 0;
+        while (i < value.Length)
+        {
+            var c = value[i];
+
+            if (char.IsHighSurrogate(c) && i + 1 < value.Length && char.IsLowSurrogate(value[i + 1]))
+            {
+                // Non-BMP pair: encode both chars as one UTF-8 sequence.
+                // Passing the pair to Encoding.UTF8 produces the correct 4-byte result
+                // rather than two replacement characters.
+                foreach (var b in Encoding.UTF8.GetBytes(new string(new[] { c, value[i + 1] })))
+                {
+                    AppendPercentEncoded(sb, b);
+                }
+
+                i += 2;
+            }
+            else
+            {
+                var shouldEncode = isKey ? !IsValidKey(c) : !IsValidValueChar(c);
+                if (shouldEncode)
+                {
+                    if (c > '\x7F')
+                    {
+                        foreach (var b in Encoding.UTF8.GetBytes(c.ToString()))
+                        {
+                            AppendPercentEncoded(sb, b);
+                        }
+                    }
+                    else
+                    {
+                        AppendPercentEncoded(sb, (byte)c);
+                    }
+                }
+                else
+                {
+                    sb.Append(c);
+                }
+
+                i++;
+            }
+        }
+#endif
+
+        return sb.ToString();
+    }
+
+    private static bool IsValidValueChar(char c) =>
+#if NET
+        ValidValueSearcher.Contains(c);
+#else
+        Array.IndexOf(ValidValueChars, c) >= 0;
+#endif
+
+    private static bool IsValidKey(char c) =>
+#if NET
+        ValidKeySearcher.Contains(c);
+#else
+        Array.IndexOf(ValidKeyChars, c) >= 0;
+#endif
+
+    private static bool IsValidKey(ReadOnlySpan<char> key)
+    {
+#if NET
+        return !key.ContainsAnyExcept(ValidKeySearcher);
 #else
-        value.IndexOfAny('%', '+') < 0 ? value.ToString() : WebUtility.UrlDecode(value.ToString());
+        foreach (var c in key)
+        {
+            if (Array.IndexOf(ValidKeyChars, c) < 0)
+            {
+                return false;
+            }
+        }
+
+        return true;
 #endif
+    }
+
+    private static string DecodeIfNeeded(ReadOnlySpan<char> value)
+    {
+#if NET
+        if (!value.ContainsAny(DecodeHints))
+        {
+            return value.ToString();
+        }
+#else
+        if (value.IndexOf('%') < 0)
+        {
+            return value.ToString();
+        }
+#endif
+
+        var sb = new StringBuilder(value.Length);
+
+        var byteBuffer = new byte[value.Length];
+        var byteCount = 0;
+        var i = 0;
+
+        while (i < value.Length)
+        {
+            if (value[i] == '%')
+            {
+                if (i + 2 < value.Length && IsHexDigit(value[i + 1]) && IsHexDigit(value[i + 2]))
+                {
+                    byteBuffer[byteCount++] = (byte)((HexDigitValue(value[i + 1]) << 4) | HexDigitValue(value[i + 2]));
+                    i += 3;
+                }
+                else
+                {
+                    FlushByteBuffer(sb, byteBuffer, ref byteCount);
+                    sb.Append('\uFFFD');
+                    i += Math.Min(3, value.Length - i);
+                }
+            }
+            else
+            {
+                FlushByteBuffer(sb, byteBuffer, ref byteCount);
+                sb.Append(value[i]);
+                i++;
+            }
+        }
+
+        FlushByteBuffer(sb, byteBuffer, ref byteCount);
+
+        return sb.ToString();
+    }
+
+    private static void FlushByteBuffer(StringBuilder sb, byte[] buffer, ref int count)
+    {
+        if (count == 0)
+        {
+            return;
+        }
+
+        sb.Append(Encoding.UTF8.GetString(buffer, 0, count));
+        count = 0;
+    }
+
+    private static bool IsHexDigit(char c) =>
+        char.IsAsciiDigit(c) || c is (>= 'A' and <= 'F') or (>= 'a' and <= 'f');
+
+    private static int HexDigitValue(char c) =>
+        c <= '9' ? c - '0' : (c & 0x0f) + 9;
+
+    private static void AppendPercentEncoded(StringBuilder sb, byte b) =>
+        sb.Append('%')
+            .Append(Hex[(b >> 4) & 0xF])
+            .Append(Hex[b & 0xF]);
 }

test/OpenTelemetry.Api.FuzzTests/Context/Propagation/BaggagePropagatorFuzzTests.cs

@@ -16,24 +16,21 @@ public class BaggagePropagatorFuzzTests
     private const int MaxBaggageItems = 180;
 
     [Property(MaxTest = MaxTests)]
-    public Property InjectExtractRoundTripPreservesSafeBaggage() => Prop.ForAll(Generators.SafeBaggageDictionaryArbitrary(), (baggageItems) =>
+    public Property ExtractNeverThrowsOnArbitraryInput() => Prop.ForAll(Generators.BaggageCarrierArbitrary(), (carrier) =>
     {
-        try
-        {
-            var propagator = new BaggagePropagator();
-            var carrier = new Dictionary<string, string>(StringComparer.Ordinal);
-            var propagationContext = new PropagationContext(default, Baggage.Create(baggageItems));
-
-            propagator.Inject(propagationContext, carrier, FuzzTestHelpers.Setter);
-
-            var extracted = propagator.Extract(default, carrier, FuzzTestHelpers.Getter);
+        var propagator = new BaggagePropagator();
+        propagator.Extract(default, carrier, FuzzTestHelpers.ArrayGetter);
+        return true;
+    });
 
-            return DictionariesEqual(baggageItems, extracted.Baggage.GetBaggage());
-        }
-        catch (Exception ex) when (FuzzTestHelpers.IsAllowedException(ex))
-        {
-            return true;
-        }
+    [Property(MaxTest = MaxTests)]
+    public Property InjectNeverThrowsOnArbitraryInput() => Prop.ForAll(Generators.BaggageDictionaryArbitrary(), (baggageItems) =>
+    {
+        var propagator = new BaggagePropagator();
+        var carrier = new Dictionary<string, string>(StringComparer.Ordinal);
+        var propagationContext = new PropagationContext(default, Baggage.Create(baggageItems));
+        propagator.Inject(propagationContext, carrier, FuzzTestHelpers.Setter);
+        return true;
     });
 
     [Property(MaxTest = MaxTests)]