[api-baggage] fix encoding of space chars in baggage item value (#5303)

Co-authored-by: Robert Pająk <[email protected]> Co-authored-by: Mikel Blanchard <[email protected]>
open-telemetry · Apr 5, 2024 · e46a446 · e46a446
1 parent dbcc511
commit e46a446
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 8 deletions.
diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+* **Breaking change:** Fix space character encoding from `+` to `%20`
+  for baggage item values when propagating baggage as defined in
+  [W3C Baggage propagation format specification](https://www.w3.org/TR/baggage/).
+  ([#5303](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5303))
+
 ## 1.8.0
 
 Released 2024-Apr-02

diff --git a/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs b/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs
@@ -1,7 +1,6 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-using System.Net;
 using System.Text;
 using OpenTelemetry.Internal;
 
@@ -94,7 +93,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(Uri.EscapeDataString(item.Key)).Append('=').Append(Uri.EscapeDataString(item.Value)).Append(',');
             }
             while (e.MoveNext() && ++itemCount < MaxBaggageItems && baggage.Length < MaxBaggageLength);
             baggage.Remove(baggage.Length - 1, 1);
@@ -141,8 +140,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))
                 {

diff --git a/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs b/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs
@@ -131,7 +131,9 @@ 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%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~";
+
         var carrier = new List<KeyValuePair<string, string>>
         {
             new KeyValuePair<string, string>(BaggagePropagator.BaggageHeaderName, initialBaggage),
@@ -142,11 +144,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 +158,10 @@ public void ValidateSpecialCharsBaggageExtraction()
 
         Assert.True(actualBaggage.ContainsKey("key()3"));
         Assert.Equal("value()!&;:", actualBaggage["key()3"]);
+
+        // x20-x7E range
+        Assert.True(actualBaggage.ContainsKey("key4"));
+        Assert.Equal(" !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~", actualBaggage["key4"]);
     }
 
     [Fact]
@@ -195,11 +201,14 @@ public void ValidateSpecialCharsBaggageInjection()
             {
                 { "key 1", "value 1" },
                 { "key2", "!x_x,x-x&x(x\");:" },
+
+                // x20-x7E range
+                { "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=%21x_x%2Cx-x%26x%28x%22%29%3B%3A,key3=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~", carrier[BaggagePropagator.BaggageHeaderName]);
     }
 }