[api-baggage] revert space encoding change (#5687)

Co-authored-by: Mikel Blanchard <mblanchard@macrosssoftware.com>
open-telemetry · Jun 12, 2024 · dc390a6 · dc390a6
1 parent 28824f8
commit dc390a6
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 16 deletions.
diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* **Breaking change:** Revert space character encoding change from `+` to `%20`
+  for baggage item values from [#5303](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5303)
+  ([#5687](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5687))
+
 ## 1.9.0-rc.1
 
 Released 2024-Jun-07

diff --git a/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs b/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs
@@ -1,6 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+using System.Net;
 using System.Text;
 using OpenTelemetry.Internal;
 
@@ -93,7 +94,7 @@ public override void Inject<T>(PropagationContext context, T carrier, Action<T,
                     continue;
                 }
 
-                baggage.Append(Uri.EscapeDataString(item.Key)).Append('=').Append(Uri.EscapeDataString(item.Value)).Append(',');
+                baggage.Append(WebUtility.UrlEncode(item.Key)).Append('=').Append(WebUtility.UrlEncode(item.Value)).Append(',');
             }
             while (e.MoveNext() && ++itemCount < MaxBaggageItems && baggage.Length < MaxBaggageLength);
             baggage.Remove(baggage.Length - 1, 1);
@@ -140,8 +141,8 @@ internal static bool TryExtractBaggage(string[] baggageCollection, out Dictionar
                     continue;
                 }
 
-                var key = Uri.UnescapeDataString(parts[0]);
-                var value = Uri.UnescapeDataString(parts[1]);
+                var key = WebUtility.UrlDecode(parts[0]);
+                var value = WebUtility.UrlDecode(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,9 +131,7 @@ public void ValidateSpecialCharsBaggageExtraction()
         Assert.Equal("key%28%293", escapedKey);
         Assert.Equal("value%28%29%21%26%3B%3A", 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 initialBaggage = $"key+1=value+1,{encodedKey}={encodedValue},{escapedKey}={escapedValue}";
         var carrier = new List<KeyValuePair<string, string>>
         {
             new KeyValuePair<string, string>(BaggagePropagator.BaggageHeaderName, initialBaggage),
@@ -144,11 +142,11 @@ public void ValidateSpecialCharsBaggageExtraction()
         Assert.False(propagationContext == default);
         Assert.True(propagationContext.ActivityContext == default);
 
-        Assert.Equal(4, propagationContext.Baggage.Count);
+        Assert.Equal(3, propagationContext.Baggage.Count);
 
         var actualBaggage = propagationContext.Baggage.GetBaggage();
 
-        Assert.Equal(4, actualBaggage.Count);
+        Assert.Equal(3, actualBaggage.Count);
 
         Assert.True(actualBaggage.ContainsKey("key 1"));
         Assert.Equal("value 1", actualBaggage["key 1"]);
@@ -158,10 +156,6 @@ 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]
@@ -201,14 +195,11 @@ 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%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]);
+        Assert.Equal("key+1=value+1,key2=!x_x%2Cx-x%26x(x%22)%3B%3A", carrier[BaggagePropagator.BaggageHeaderName]);
     }
 }