[baggage] baggage item value encoding fix - custom encoder

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

@@ -1,8 +1,8 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-using System.Net;
 using System.Text;
+
 using OpenTelemetry.Internal;
 
 namespace OpenTelemetry.Context.Propagation;
@@ -94,7 +94,7 @@ public override void Inject<T>(PropagationContext context, T carrier, Action<T,
                     continue;
                 }
 
-                baggage.Append(WebUtility.UrlEncode(item.Key)).Append('=').Append(WebUtility.UrlEncode(item.Value)).Append(',');
+                baggage.Append(W3CBaggageValueEncoder.Encode(item.Key)).Append('=').Append(W3CBaggageValueEncoder.Encode(item.Value)).Append(',');
             }
             while (e.MoveNext() && ++itemCount < MaxBaggageItems && baggage.Length < MaxBaggageLength);
             baggage.Remove(baggage.Length - 1, 1);
@@ -141,8 +141,8 @@ internal static bool TryExtractBaggage(string[] baggageCollection, out Dictionar
                     continue;
                 }
 
-                var key = WebUtility.UrlDecode(parts[0]);
-                var value = WebUtility.UrlDecode(parts[1]);
+                var key = Uri.UnescapeDataString(parts[0]);
+                var value = Uri.UnescapeDataString(parts[1]);
 
                 if (string.IsNullOrEmpty(key) || string.IsNullOrEmpty(value))
                 {

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

@@ -0,0 +1,120 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+using System.Runtime.CompilerServices;
+using System.Text;
+
+#nullable enable
+
+namespace OpenTelemetry.Context.Propagation;
+
+// Encodes baggage values according to the https://www.w3.org/TR/baggage/#value spec.
+// This is a modified code of WebUtility.Encode, which handles space char differently -
+// it is percent-encoded, instead of being converted to '+'.
+// Additionally, allowed characters from baggage-octet range from the spec are not percent-encoded.
+// Encoding baggage value with this encoder yields minimal, spec-compliant representation.
+
+internal static class W3CBaggageValueEncoder
+{
+    // originated from https://github.com/dotnet/runtime/blob/9429e432f39786e1bcd1c080833e5d7691946591/src/libraries/System.Private.CoreLib/src/System/Net/WebUtility.cs#L368
+
+    // Modified to percent-encode space char instead of converting it to '+'.
+    public static string? Encode(string? value)
+    {
+        if (string.IsNullOrEmpty(value))
+        {
+            return value;
+        }
+
+        int safeCount = 0;
+        for (int i = 0; i < value!.Length; i++)
+        {
+            char ch = value[i];
+            if (!RequiresPercentEncoding(ch))
+            {
+                safeCount++;
+            }
+        }
+
+        if (safeCount == value.Length)
+        {
+            // Nothing to expand
+            return value;
+        }
+
+        int byteCount = Encoding.UTF8.GetByteCount(value);
+        int unsafeByteCount = byteCount - safeCount;
+        int byteIndex = unsafeByteCount * 2;
+
+        // comment originated from https://github.com/dotnet/runtime/blob/9429e432f39786e1bcd1c080833e5d7691946591/src/libraries/System.Private.CoreLib/src/System/Net/WebUtility.cs#L405
+
+        // Instead of allocating one array of length `byteCount` to store
+        // the UTF-8 encoded bytes, and then a second array of length
+        // `3 * byteCount - 2 * unexpandedCount`
+        // to store the URL-encoded UTF-8 bytes, we allocate a single array of
+        // the latter and encode the data in place, saving the first allocation.
+        // We store the UTF-8 bytes to the end of this array, and then URL encode to the
+        // beginning of the array.
+        byte[] newBytes = new byte[byteCount + byteIndex];
+        Encoding.UTF8.GetBytes(value, 0, value.Length, newBytes, byteIndex);
+
+        GetEncodedBytes(newBytes, byteIndex, byteCount, newBytes);
+        return Encoding.UTF8.GetString(newBytes);
+    }
+
+    private static bool RequiresPercentEncoding(char ch)
+    {
+        // The percent char MUST be percent-encoded.
+        return ch == '%' || !IsInBaggageOctetRange(ch);
+    }
+
+    private static bool IsInBaggageOctetRange(char ch)
+    {
+        // from the spec:
+        // Any characters outside of the baggage-octet range of characters MUST be percent-encoded.
+        // baggage-octet          =  %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
+        return ch == '!' || // 0x21
+               (ch >= '#' && ch <= '+') || // 0x23-0x2B
+               (ch >= '-' && ch <= ':') || // 0x2D-0x3A
+               (ch >= '<' && ch <= '[') || // 0x3C-0x5B
+               (ch >= ']' && ch <= '~');   // 0x5D-0x7E
+    }
+
+    // originated from https://github.com/dotnet/runtime/blob/9429e432f39786e1bcd1c080833e5d7691946591/src/libraries/System.Private.CoreLib/src/System/Net/WebUtility.cs#L328
+    // Modified to percent-encode the space char.
+    private static void GetEncodedBytes(byte[] originalBytes, int offset, int count, byte[] expandedBytes)
+    {
+        int pos = 0;
+        int end = offset + count;
+        for (int i = offset; i < end; i++)
+        {
+            byte b = originalBytes[i];
+            char ch = (char)b;
+            if (RequiresPercentEncoding(ch))
+            {
+                expandedBytes[pos++] = (byte)'%';
+                expandedBytes[pos++] = (byte)ToCharUpper(b >> 4);
+                expandedBytes[pos++] = (byte)ToCharUpper(b);
+            }
+            else
+            {
+                expandedBytes[pos++] = b;
+            }
+        }
+    }
+
+    // originated from https://github.com/dotnet/runtime/blob/9429e432f39786e1bcd1c080833e5d7691946591/src/libraries/Common/src/System/HexConverter.cs#L203
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private static char ToCharUpper(int value)
+    {
+        value &= 0xF;
+        value += '0';
+
+        if (value > '9')
+        {
+            value += 'A' - ('9' + 1);
+        }
+
+        return (char)value;
+    }
+}

test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs

@@ -131,7 +131,8 @@ public void ValidateSpecialCharsBaggageExtraction()
         Assert.Equal("key%28%293", escapedKey);
         Assert.Equal("value%28%29%21%26%3B%3A", escapedValue);
 
-        var initialBaggage = $"key+1=value+1,{encodedKey}={encodedValue},{escapedKey}={escapedValue}";
+        var initialBaggage =
+            $"key%201=value%201,{encodedKey}={encodedValue},{escapedKey}={escapedValue},key4=%20!%22#$%25&'()*+%2C-./0123456789:%3B<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[%5C]^_`abcdefghijklmnopqrstuvwxyz{{|}}~";
         var carrier = new List<KeyValuePair<string, string>>
         {
             new KeyValuePair<string, string>(BaggagePropagator.BaggageHeaderName, initialBaggage),
@@ -142,11 +143,11 @@ public void ValidateSpecialCharsBaggageExtraction()
         Assert.False(propagationContext == default);
         Assert.True(propagationContext.ActivityContext == default);
 
-        Assert.Equal(3, propagationContext.Baggage.Count);
+        Assert.Equal(4, propagationContext.Baggage.Count);
 
         var actualBaggage = propagationContext.Baggage.GetBaggage();
 
-        Assert.Equal(3, actualBaggage.Count);
+        Assert.Equal(4, actualBaggage.Count);
 
         Assert.True(actualBaggage.ContainsKey("key 1"));
         Assert.Equal("value 1", actualBaggage["key 1"]);
@@ -156,6 +157,9 @@ public void ValidateSpecialCharsBaggageExtraction()
 
         Assert.True(actualBaggage.ContainsKey("key()3"));
         Assert.Equal("value()!&;:", actualBaggage["key()3"]);
+
+        Assert.True(actualBaggage.ContainsKey("key4"));
+        Assert.Equal(" !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~", actualBaggage["key4"]);
     }
 
     [Fact]
@@ -195,11 +199,12 @@ public void ValidateSpecialCharsBaggageInjection()
             {
                 { "key 1", "value 1" },
                 { "key2", "!x_x,x-x&x(x\");:" },
+                { "key3", " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~" },
             }));
 
         this.baggage.Inject(propagationContext, carrier, Setter);
 
         Assert.Single(carrier);
-        Assert.Equal("key+1=value+1,key2=!x_x%2Cx-x%26x(x%22)%3B%3A", carrier[BaggagePropagator.BaggageHeaderName]);
+        Assert.Equal("key%201=value%201,key2=!x_x%2Cx-x&x(x%22)%3B:,key3=%20!%22#$%25&'()*+%2C-./0123456789:%3B<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[%5C]^_`abcdefghijklmnopqrstuvwxyz{|}~", carrier[BaggagePropagator.BaggageHeaderName]);
     }
 }